refactor(template-compiler): move implicit stylesheet import and style token to template compiler #4147
Conversation
| } | ||
|
|
||
| const compiled = compiler(src, config); | ||
| const compiled = compiler(src, filename, config); |
There was a problem hiding this comment.
The new snapshots all have the implicit imports, which can be noisey.
As an alternative we could add something to cut out the imports for certain snapshots.
I opted to leave it as-is for now to get opinions.
There was a problem hiding this comment.
I'm fine with the snapshots being a bit noisy. I actually prefer to see what the code actually looks like, rather than trying to hide boilerplate/repetition.
| const relPath = `./${path.basename(filename, path.extname(filename))}`; | ||
| const imports = IMPLICIT_STYLESHEET_IMPORTS.map((stylesheet) => { | ||
| const extension = stylesheet === IMPLICIT_STYLESHEETS ? '.css' : '.scoped.css?scoped=true'; | ||
| return t.importDeclaration( | ||
| [t.importDefaultSpecifier(t.identifier(stylesheet))], | ||
| t.literal(`${relPath}${extension}`) | ||
| ); | ||
| }); | ||
|
|
There was a problem hiding this comment.
The reason we need the filename now is for these implicit imports
| */ | ||
| scopeTokens: scopeTokens; | ||
|
|
||
| constructor(config: NormalizedConfig, filename: string) { |
There was a problem hiding this comment.
I understand why the filename isn't part of the config, but it strikes me as a bit odd... might be worth a comment.
There was a problem hiding this comment.
Updated in the latest commit! - 85f8a18
There was a problem hiding this comment.
This LGTM, although it is technically a breaking change, so we should save it for the 7.0.0 milestone.
I also opened some follow-up PRs for random off-topic things: #4151 #4149
I'm also astonished that the downstream LWR tests are passing. It would be good to do a sanity check of this change in LWR to make sure everything works correctly. (I suspect HMR may be broken, for instance.)
FYI @lpomerleau @kmbauer
|
I compared the nucleus downstream test failures against the latest commit in There will be a snapshot test failure for |
Details
Fixes #3931
This will be a breaking change, the
compileAPI now requires the file name, which is used to determine the css imports.I've also expanded the config to accept
namespaceandname.This change updates all the template compiler snapshots and I've separated them into 2 commits;
Does this pull request introduce a breaking change?
The
@lwc/template-compilercompilefunction now requires the component's file name when compiling the template.Does this pull request introduce an observable change?
GUS work item
W-15461766