Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ssr-compiler): implement scoped styles and scope tokens #4567

Merged
merged 7 commits into from
Sep 26, 2024

Conversation

nolanlawson
Copy link
Collaborator

@nolanlawson nolanlawson commented Sep 20, 2024

Details

This reduces the number of test failures in fixtures.spec.ts from 60 to 55. It implements partial support for *.scoped.css as well as the scope tokens (classes) that are supposed to be applied during HTML rendering, e.g.:

<x-basic class="lwc-1rssj1tib70-host">
  <style class="lwc-1rssj1tib70" type="text/css">
    p.lwc-1rssj1tib70 {background-color: blue;}
  </style>
  <p class="lwc-1rssj1tib70">
    Hello
  </p>
</x-basic>

💁‍♂️ Reviewer pro tip! This PR won't make a lick of sense unless you look at the actual compiled-experimental-ssr.js files from the fixture tests. Here is a diff: 62d123f

You can run also run the fixtures tests yourself:

TEST_SSR_COMPILER=1 yarn test ./packages/@lwc/ssr-compiler/src/__tests__/fixtures.spec.ts

Does this pull request introduce a breaking change?

  • 😮‍💨 No, it does not introduce a breaking change.

Does this pull request introduce an observable change?

  • 🤞 No, it does not introduce an observable change.

GUS work item

W-16782117

@nolanlawson nolanlawson requested a review from a team as a code owner September 20, 2024 20:47
yield \`<\${tagName}\`;
yield tmplFn.stylesheetScopeTokenHostClass;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scope tokens are associated with templates, not with components. This is the main mistake that the previous code was making. Here I am grabbing the scope token from the tmplFn, and I'm also not passing it into the tmplFn below because the tmplFn should already know what its stylesheets are.

(This is not true for static stylesheets but we'll cross that bridge when we get there.)

)}`
);
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just moved this error message from elsewhere to here.

esTemplate<ExportNamedDeclaration>`
const stylesheetScopeTokenClassPrefix = hasScopedStylesheets ? (stylesheetScopeToken + ' ') : '';
`,
];
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is probably a better way to do this, but I like the esTemplate utility and did not find a more succinct way to represent this.

@@ -24,6 +24,7 @@ export { CustomRendererConfig, CustomRendererElementConfig } from './shared/rend
export { Config } from './config';
export { toPropertyName } from './shared/utils';
export { kebabcaseToCamelcase } from './shared/naming';
export { generateScopeTokens } from './scopeTokens';
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are now exposing this from the template compiler... may the gods forgive us.

* @param filename - full filename, e.g. `path/to/x/foo/foo.html`
* @param namespace - namespace, e.g. 'x' for `x/foo/foo.html`
* @param componentName - component name, e.g. 'foo' for `x/foo/foo.html`
*/
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a big warning here that this is not stable. I can't imagine why someone would use it, but you never know... 🤷

@nolanlawson nolanlawson merged commit edf0d7c into master Sep 26, 2024
11 checks passed
@nolanlawson nolanlawson deleted the nolan/ssr-default-style branch September 26, 2024 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants