fix: updated toolkit to use .module.scss pathing #60
Conversation
dalebaldwin
requested review from
sentience,
dougmacknz,
shreyasbharath and
HeartSquared
🦋 Changeset detectedLatest commit: 460afb1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
package.json
Outdated
| @@ -26,7 +26,7 @@ | |||
| "scripts": { | |||
| "build": "tsc --build && yarn copy-files", | |||
| "clean": "tsc --build --clean", | |||
| "copy-files": "copyfiles -u 1 src/**/*.scss dist/", | |||
| "copy-files": "copyfiles -u 1 src/**/*.module.scss dist/", | |||
There was a problem hiding this comment.
*.module.scss will still be matched against *.scss so this isn't necessary :)
It would actually be safer to leave it in case there are internal scss files (eg. mixins consumed by a file with .module.scss) which are valid.
types.d.ts
Outdated
| @@ -1,4 +1,4 @@ | |||
| declare module "*.scss" { | |||
| declare module "*.module.scss" { | |||
There was a problem hiding this comment.
Same reasoning for not changing the script. The regex still applies.
There was a problem hiding this comment.
Makes sense, I'll switch them back
There was a problem hiding this comment.
I wonder if this is actually a good change because if you import a .scss file and don't get types for it, that would be a warning sign that you're importing CSS that won't be processed as a CSS module, because you didn't name it .module.scss.
In fact, I'm thinking we might even want to declare module "*.scss" never or something like that as a further guard against importing CSS files that aren't CSS modules.
Perhaps this is overcomplicating things, though.
There was a problem hiding this comment.
Certainly an interesting proposal. The two declarations may fight each other though (haven't tested, so could be wrong). But given this is only a problem because of the combination of this repo not compiling its own styles + Next being opinionated on the filename, it's not necessarily a long term restriction we require.
KAIO for instance compiles its own styles, so this problem would never exist even if we were to name them all without .module.
Given we're looking into potential refactors to RTE + RTT for it to either live or be compatible with KAIO, it's better to leave implementing this for later (if at all) :)
|
@HeartSquared @sentience who has merge permissions on this? |
|
|
We discovered in our React 18, next js project that the popovers weren't rendering with the correct CSS. This issue was tracked back to the
.scssfile extension needing to be renamed to.module.scss.We will need to do an additional canary release on the R18 version of the RTE to get this issue resolved as well.