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

bug: Stencil dev server CSS updates are broken when using dist-hydrate-script #3461

Closed
3 tasks done
alicewriteswrongs opened this issue Jul 8, 2022 · 2 comments
Closed
3 tasks done
Labels
Bug: Validated This PR or Issue is verified to be a bug within Stencil

Comments

@alicewriteswrongs
Copy link
Contributor

alicewriteswrongs commented Jul 8, 2022

Prerequisites

Stencil Version

2.17.0

Current Behavior

summary: if dist-hydrate-script output target is set then CSS hot-reload is broken and changing a CSS file will result in components losing all of their styling.

This problem has to do with how we rewrite CSS in order to support scoped components, an alternative way to scope styles to a component which does not use the shadow DOM. We have a utility function, defined in src/utils/shadow-css.ts, which rewrites a CSS string in order to 'scope' it within a component by rewriting the selectors to include a unique ID. This function is called in the dev server workflow in such a way that CSS gets mangled if (possibly iff):

  • a component has shadow: true and
  • the dist-hydrate-script output target is set in the Stencil configuration

This results in styles targeting the :host selector to be rewritten to a different selector, wiping styles from the component when the updated CSS is applied to the page in the browser.

Why is this happening?

The scopeCss function is defined here:

export const scopeCss = (cssText: string, scopeId: string, commentOriginalSelector: boolean) => {

This function is called when the dev server is processing a change event to a CSS file. The function is called in the transformCssToEsmModule function in src/compiler/style/css-to-esm.ts:

if (isString(input.tag)) {
if (input.encapsulation === 'scoped' || (input.encapsulation === 'shadow' && input.commentOriginalSelector)) {
const scopeId = getScopeId(input.tag, input.mode);
results.styleText = scopeCss(results.styleText, scopeId, input.commentOriginalSelector);
}
}

The third argument to the function is here set to input.commentOriginalSelector, which causes the CSS selectors to be commented out (see below). This causes CSS using the :host selector for styling the component's shadow DOM to be mangled in such a way that the styles don't apply to anything.

The third argument is set in the extTransformsPlugin, here:

const cssTransformResults = await compilerCtx.worker.transformCssToEsm({
file: pluginTransforms.id,
input: pluginTransforms.code,
tag: data.tag,
encapsulation: data.encapsulation,
mode: data.mode,
commentOriginalSelector,
sourceMap: config.sourceMap,
minify: config.minifyCss,
autoprefixer: config.autoprefixCss,
docs: config.buildDocs,
});

and here:

const commentOriginalSelector = bundleOpts.platform === 'hydrate' && data.encapsulation === 'shadow';

bundleOpts.platform is in turn set to "hydrate" in the BundleOptions object constructed in bundleHydrateFactory, here:

const bundleOpts: BundleOptions = {
id: 'hydrate',
platform: 'hydrate',
conditionals: getHydrateBuildConditionals(buildCtx.components),
customTransformers: getHydrateCustomTransformer(config, compilerCtx),
inlineDynamicImports: true,
inputs: {
'@app-factory-entry': '@app-factory-entry',
},
loader: {
'@app-factory-entry': appFactoryEntryCode,
},
};

which is in turn called in the generateHydrateApp function which does the main work for the dist-hydrate-script output target:

const rollupFactoryBuild = await bundleHydrateFactory(config, compilerCtx, buildCtx, build, appFactoryEntryCode);

export const outputHydrateScript = async (config: d.Config, compilerCtx: d.CompilerCtx, buildCtx: d.BuildCtx) => {
const hydrateOutputTargets = config.outputTargets.filter(isOutputTargetHydrate);
if (hydrateOutputTargets.length > 0) {
const timespan = buildCtx.createTimeSpan(`generate hydrate app started`);
await generateHydrateApp(config, compilerCtx, buildCtx, hydrateOutputTargets);
timespan.finish(`generate hydrate app finished`);
}
};

Anyhow, to make a long story short, when the dist-hydrate-script output target is set then CSS will be 'scoped' using the scopeCss function with the commentOriginalSelector argument set to true, even in situations in which it isn't appropriate to do so, and in particular, when a component does not have scoped: true and instead has shadow: true. This will cause CSS like:

:host {
    background: blue;
}

to be rewritten like

/*!@:host*/.my-component-h{background: blue;}

This, in turn, results in the CSS not applying to the component, and we get a nice, unstyled component instead of what we want.

Why doesn't this happen on the first page load?

I believe this doesn't happen on first page load because on first page load CSS is processed in the initializeComponent function but in that instance it is not called with the commentOriginalSelector argument set to true:

if (
!BUILD.hydrateServerSide &&
BUILD.shadowDom &&
BUILD.shadowDomShim &&
cmpMeta.$flags$ & CMP_FLAGS.needsShadowDomShim
) {
style = await import('../utils/shadow-css').then((m) => m.scopeCss(style, scopeId, false));
}

Expected Behavior

The hot module reload functionality in the dev server should work, i.e. edits made in the CSS file for a component should be reflected in the browser automatically without having to reload the page.

Steps to Reproduce

There are detailed instructions in the reproduction repo.

Code Reproduction URL

https://github.com/alicewriteswrongs/stencil-hmr-hydrate-repro

Additional Information

No response

@ionitron-bot ionitron-bot bot added the triage label Jul 8, 2022
@rwaskiewicz rwaskiewicz added Bug: Validated This PR or Issue is verified to be a bug within Stencil and removed triage labels Jul 8, 2022
@alicewriteswrongs
Copy link
Contributor Author

The offending line (the encapsulation === "shadow" check) was added in this commit in 2020: b5757df

const commentOriginalSelector = (bundleOpts.platform === 'hydrate') && (data.encapsulation === 'shadow');

afaict it isn't necessary? there is no context in the original change other than the commit message "only comment original selectors for shadow css."

pull bot pushed a commit to digitty-forks/stencil that referenced this issue Aug 16, 2022
This fixes an issue (ionic-team#3461) where CSS is being inappropriately mangled
when using the hot-reload dev server. The issue has details about
exactly what the problem is, but basically in short if you are using the
`dist-hydrate-script` output target and running the dev server then any
change to a CSS file will cause all styling to be wiped from the related
component in the browser, making for a pretty inadequate developer
experience.

This commit fixes the issue by changing the conditions under which
original CSS selectors are commented out
@rwaskiewicz
Copy link
Member

The PR that fixes this issue (#3517) is a part of today's v2.17.4 release. As a result, I'm going to close this issue. Should the issue reoccur on v2.17.4+ of Stencil, please open a new ticket, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug: Validated This PR or Issue is verified to be a bug within Stencil
Projects
None yet
Development

No branches or pull requests

2 participants