fix(style): fixes to watching nested and multiple styles on Stencil components

[christian-bromann]

This PR contains a set of small optimizations and bugfixes. I originally wanted to keep them in separate PRs but then found that they were small enough to be combined in one. The following issues get addressed:

Multiple style urls aren't recognised in watch mode

fixes #5251

When a component is bundled with multiple css files attached to it, the output will look as following:

const Button = class {
    constructor(hostRef) {
        registerInstance(this, hostRef);
    }
    render() {
        return h("button", { class: "button" }, "Button Text ", h("span", null, "OK!"));
    }
};
Button.style = buttonBackgroundCss + buttonTextCss;

From this Stencil creates a style tag within the shadow DOM to enable style HMR and injects the combined styles of the component cmp.style. Now when we update one CSS file, we push the update to the build context buildCtx.stylesUpdated which will cause in replacing the content with what is in stylesUpdated. Unfortunately, when we change one file, we only get the update content of that file and not the combined styles.

The patch stores the transform results of styles and is able to send a concatenation of them to the application.

Changes were applied in src/compiler/bundle/ext-transforms-plugin.ts

Review Guide

To review this change:

• run git clone https://github.com/a7medm7med/stencil-bug
• cd stencil-bug && npm i
• npm start
• in src/components/button/button.text.scss chnage color from red to e.g. blue and verify that: background color is reseted and only styles from src/components/button/button.text.scss are applied to the page

to verify fix:

• install dev build: npm i @stencil/core@4.9.1-dev.1705446396.3c9fd4d
• restart dev server
• change color in src/components/button/button.text.scss and verify that background color remains red 🎉

nested files are not applied in watch mode

fixes #2635
fixes #2741
fixes #1908
fixes #1388

it also:
closes #1909 (attempt to fix this bug in a different way which turned out to be incorrect)

If a component references an external style, e.g.:

@Component({
  tag: 'my-component',
  styleUrl: 'my-component.css',
  shadow: true,
})
export class MyComponent {
  // ...
}

or a global style is defined which imports another sass file:

// my-component.css
@import "./foo.scss";

div {
  background-color: var(--body-bg);
}

:host {
  --body-bg: blue;
}

then changes in foo.scss will not be recognised and user has to restart the dev server.

Changes applied in src/compiler/plugin/plugin.ts fix this problem. They were adopted from #3110 and basically just add detected file dependencies to the transform result.

Review Guide

To review this change:

• run git clone git@github.com:emily-ut/stencil-playground.git
• cd stencil-playground
• git checkout bug-scss-watcher
• npm i
• npm start
• change css variable in src/global/theme/theme.wireframe.scss to a different color, verify that background color doesn't change

to verify fix:

• install dev build: npm i @stencil/core@4.9.1-dev.1705446396.3c9fd4d
• restart dev server
• change css variable in src/global/theme/theme.wireframe.scss to a different color, verify that background now gets updated correctlt 🎉

Styles outside of project dir aren't picked up by watcher

Currently we add only the files that are within config.srcDir to our watch process. If a file is outside of that directory, it won't get picked up. As a fix I added a check to the extTransformsPlugin which touches all styles that are used by any of the components and will add a style if the path is relative or absolute and not within the srcDir.

Some considerations:

• there might be a better place to add a style to the watch list
• we might not want to tweak this at runtime and rather have the user specify other watch folders

Review Guide

To review this change:

• run git clone git@github.com:James-Wilkinson-git/stencil-bug.git
• cd ./stencil-bug/stencil-install/stencil
• npm i
• npm start
• change color in ./stencil-bug/shared-style/my-component.scss to a different color, verify that background color doesn't change

to verify fix:

• cd ./stencil-bug/stencil-install/stencil
• install dev build: npm i @stencil/core@4.9.1-dev.1705446396.3c9fd4d
• restart dev server
• change color in ./stencil-bug/shared-style/my-component.scss to a different color, verify that background now gets updated correctlt 🎉

[github-actions]

--strictNullChecks error report

Typechecking with --strictNullChecks resulted in 1209 errors on this branch.

That's 1 fewer than on main! 🎉🎉🎉

reports and statistics

Our most error-prone files

Path	Error Count
src/dev-server/index.ts	37
src/mock-doc/serialize-node.ts	36
src/dev-server/server-process.ts	32
src/compiler/prerender/prerender-main.ts	22
src/testing/puppeteer/puppeteer-element.ts	22
src/runtime/client-hydrate.ts	20
src/screenshot/connector-base.ts	19
src/runtime/vdom/vdom-render.ts	17
src/compiler/config/test/validate-paths.spec.ts	16
src/dev-server/request-handler.ts	15
src/compiler/prerender/prerender-optimize.ts	14
src/compiler/sys/stencil-sys.ts	14
src/compiler/transpile/transpile-module.ts	14
src/sys/node/node-sys.ts	14
src/compiler/prerender/prerender-queue.ts	13
src/compiler/sys/in-memory-fs.ts	13
src/runtime/connected-callback.ts	13
src/runtime/set-value.ts	13
src/compiler/output-targets/output-www.ts	12
src/compiler/transformers/test/parse-vdom.spec.ts	12

Our most common errors

Typescript Error Code	Count
TS2345	366
TS2322	362
TS18048	224
TS18047	99
TS2722	37
TS2532	26
TS2531	23
TS2454	14
TS2352	12
TS2790	11
TS2769	8
TS2538	8
TS2344	6
TS2416	6
TS2493	3
TS18046	2
TS2684	1
TS2430	1

Unused exports report

There are 14 unused exports on this PR. That's the same number of errors on main, so at least we're not creating new ones!

Unused exports

File	Line	Identifier
src/runtime/bootstrap-lazy.ts	21	setNonce
src/screenshot/screenshot-fs.ts	18	readScreenshotData
src/testing/testing-utils.ts	198	withSilentWarn
src/utils/index.ts	145	CUSTOM
src/utils/index.ts	269	normalize
src/utils/index.ts	7	escapeRegExpSpecialCharacters
src/compiler/app-core/app-data.ts	25	BUILD
src/compiler/app-core/app-data.ts	115	Env
src/compiler/app-core/app-data.ts	117	NAMESPACE
src/compiler/fs-watch/fs-watch-rebuild.ts	123	updateCacheFromRebuild
src/compiler/types/validate-primary-package-output-target.ts	61	satisfies
src/compiler/types/validate-primary-package-output-target.ts	61	Record
src/testing/puppeteer/puppeteer-declarations.ts	485	WaitForEventOptions
src/compiler/sys/fetch/write-fetch-success.ts	7	writeFetchSuccessSync

[a7medm7med]

@christian-bromann
Great effort on addressing the issue related to the @import rule! However, as expected, the fix appears to be specific to @import and does not extend to the @use rules. Unfortunately, nested files still aren't being applied in watch mode.

For a detailed look at the issue, please refer to the repository linked below:
https://github.com/a7medm7med/stencil-bug-2

[christian-bromann]

@a7medm7med thanks for the feedback, I will look into this starting next week!

[christian-bromann]

For a detailed look at the issue, please refer to the repository linked below:

@a7medm7med I am not able to reproduce this. If I install the project and run npm start I can update both sass files and changes get directly applied.

[a7medm7med]

@christian-bromann, kindly follow these steps to address the issue:

• Run the command npm start in your terminal.
• Navigate to the button.background.scss file and modify the background color to a different shade.
• Observe that the background color does not update on the development server.

If you encounter any difficulties reproducing the issue, I am prepared to provide a video demonstration for clarification. Please let me know if further assistance is required.

[christian-bromann]

Please provide a video as I am unable to reproduce this, see:

[a7medm7med]

VID-20240116-WA0000.mp4

[christian-bromann]

@a7medm7med can you verify if this is an identical issue to what you have reported in #4130?

[a7medm7med]

@christian-bromann
Yes, I can confirm that this is an identical issue to what I reported in #4130. I've tested it on Linux, and the problem does not occur there. It seems to be specific to Windows.

[rwaskiewicz]

In the future, can you please separate these into separate PRs? Based on the PR summary, we're fixing three separate issues here:

1. Multiple style urls aren't recognised in watch mode
2. nested files are not applied in watch mode
3. Styles outside of project dir aren't picked up by watcher

While the changelist is small in size, I've found that it helps immensely in Stencil to be able to differentiate which changes are for which fixes - this helps with git bisecting, backing out changes if needed, and git blame (to figure out why something is needed/where it came from). No action required here (I think I have a good idea as to what's going on here based on yesterday's jam session), just something to keep in mind for future efforts.

[christian-bromann]

@rwaskiewicz thanks for the feedback. I addressed the comments.

[rwaskiewicz]

I found an issue with the test case - Styles outside of project dir aren't picked up by watcher. I wasn't able to verify this fix for this scenario. I tried to create something myself to make sure I wasn't misunderstanding the repro, and hit the same error with both the instructions in the PR summary and those below:

1. I created a new Stencil component starter with npm init stencil@latest component css-test in my /tmp directory, and installed a local build of this branch based on f020caf (npm run clean && npm ci && npm run build && npm pack)
2. I created /tmp/outside.css, whose content is:

:host {
   color: red;
}

(also in the lower right hand pane in the terminal in the video below).
3. I updated the styles for my-component.tsx to use the new styles outside of src:

@Component({
  tag: 'my-component',
-  styleUrl: 'my-component.css',
+  styleUrls: ['my-component.css', '../../../../outside.css'],
  shadow: true,
})

4. I started the dev server, npm start
5. On updating /tmp/outside.css (changing the value for color), it appears there's a refresh needed for styles to propagate:

Screen.Recording.2024-01-19.at.10.21.09.AM.mov

[tanner-reits]

I think I'd personally lean toward a config option that allows devs to specify additional files/dirs to watch (like a glob pattern array). But, if I'm the only one then this works too!

[christian-bromann]

@rwaskiewicz I found this error be particularly only when you link the Stencil project. Will look into fixing it.

[christian-bromann]

@rwaskiewicz found a fix, please try again.

[rwaskiewicz]

@christian-bromann For posterity, I was able to replicate this with a dev build (4.9.1-dev.1705676640.f020caf) as well. Looks like it's working now, TAL at the changes now

[rwaskiewicz]

Prettier is failing - non-blocking though for this approval.

I don't have a strong opinion on whether a user should specify the directory or not. I'm OK with trying this approach (autodiscovery) out for now as I think that's what folks will come to expect. If we can find evidence it's a big footgun, we can always revisit

[christian-bromann]

This patch has been released as a part of today's Stencil v4.11.0 release.

[silvertech-daniel]

This works perfectly so far for components, thanks! One problem I'm noticing is globalStyle dependency .scss files are being noticed (it triggers a rebuild) but not actually including the changes unless I touch the globalStyle file. Another is that a component without a styleUrl will be rebuilt when unrelated styles are updated.

[christian-bromann]

Interesting, that seems like a related but somewhat new issue. Mind raising a new issue for this with some detailed description of what you would expect and how it is actually behaving? From there we can inject this into our backlog and address accordingly. Thank you!