ionic-team / stencil

A toolchain for building scalable, enterprise-ready component systems on top of TypeScript and Web Component standards. Stencil components can be distributed natively to React, Angular, Vue, and traditional web developers from a single, framework-agnostic codebase.
https://stenciljs.com
Other
12.55k stars 784 forks source link

Styles from nested scss-files are not applied in watch mode #2635

Closed jgroth closed 9 months ago

jgroth commented 4 years ago

Stencil versions:

 @stencil/core@1.17.3
 @stencil/core@2.0.0-1

I'm submitting a:

[ x ] bug report [ ] feature request [ ] support request => Please do not submit support requests here, use one of these channels: https://stencil-worldwide.herokuapp.com/ or https://forum.ionicframework.com/

Current behavior: If a component includes styles in a .scss file and that file in turn imports a second file, changes in the second file are not applied when running the dev server.

Expected behavior: Changes in nested files are applied when running the dev server

Steps to reproduce:

The watcher seem to trigger a new build and the dev server reloads the current page with components as expected, but the result does not include the changes. Reloading the page does not work either. The following seems to work as a workaround, but is annoying

It also seem to work when using .css files instead of .scss

Related code:

// my-component.tsx
@Component({
    tag: 'my-component',
    shadow: true,
    styleUrl: 'style.scss'
})
export class MyComponent {
    render() {
        return (
            <div>
                <h1>Hello</h1>
                <h2>World</h2>
            </div>
        );
    }
}
// style.scss
@import './style2.scss';

h1 {
    color: blue;
}
// style2.scss
h2 {
    color: orange;
}
acodesmith commented 3 years ago

As a work around:

  rollupPlugins: {
    before: [
      {
        name: 'watch-external',
        async buildStart() {
          const styleFiles = await fg(resolve(__dirname, './components/**/*.scss'));
          for (let file of styleFiles) {
            this.addWatchFile(file);
          }
        },
      },
    ],
  },

Note: I'm using the node fs resolve function and a library called fast-glob (aka fg). The code snippet is added to the stencil.config.js

markcellus commented 3 years ago

The workaround mentioned in https://github.com/ionic-team/stencil/issues/2635#issuecomment-768608841 doesn't seem to work for me. But I'm not using the latest Stencil version. Can anyone confirm if this is an existing issue in the latest Stencil version?

gfellerph commented 3 years ago

@markcellus I'm using the 2.7.0 core and this issue brought me here.

dgonzalezr commented 3 years ago

@markcellus @tuelsch I have been facing the same issue for months, and this was the workaround that worked for me, hope it is helpful. Ideally, it will be good if it is solved from stencil-sass.

4aficiona2 commented 3 years ago

This is also what we experience and it isn't a great DX to always restart the build process to see the nested Sass changes reflected while development.

It's a must to have a decent DX and should definitely be part of the default setup without tweaks or workarounds.

Global nested/split Sass Files help immensly to structure the Sass code (normalize, typography, theming) instead of handling this within one big file.

tricki commented 3 years ago

I just did some debugging and found a possible cause for this bug. This line checks if any of the changed style files are imported in the global style, but it looks for @import statements in the compiled style (compilerCtx.cachedGlobalStyle) where those @imports are already replaced with the file's content (for sass files).

Not sure how to fix it though. Maybe not use the cached style to look for @imports but that would probably slow down the build even for non-sass users. A simpler option might be for @stencil/sass to add a comment above the included file's content (not sure if that's possible with node-sass) which the watcher can read, but that would require this line stripping out the comments to be changed.

FYI The simplest workaround I found is simply saving the global scss file without any changes.

johnjenkins commented 3 years ago

hey! I decided to see if I could help with this issue but unfortunately I couldn't recreate. Any pointers? This is the repo This is the result

johnjenkins commented 3 years ago

ok - I recreated using globalStyle ... perhaps issue description needs updating or a new ticket needs creating

tfrijsewijk commented 2 years ago

@johnjenkins I think the stencil's that are used in the opening post are quite old (1.something and 2.0.0-1).

On the other hand, I still have the same issue:

My component stylesheet stencil-library/components/alert/alert.scss imports @some-package/styles/stylesheet.scss. Changes in the latter are never applied to the component stylesheet unless:

I tried the workaround and enableCache: false in stencil.config.ts, but no joy. Files are watched, changes trigger a rebuild.. but nothing happens.

johnjenkins commented 2 years ago

@tfrijsewijk but if you see the original video (here) / repo I posted - with that setup you describe I could't recreate. I could only recreate with globalStyle

tfrijsewijk commented 2 years ago

Your video setup has a different directory layout (which also works for me, btw):

app-home.tsx
once.scss
two.scss

My setup has a more complex setup:

/packages/core/components/alert/alert.tsx
/packages/core/components/alert/alert.scss
/packages/sources/components/alert/alert.scss

This is a Yarn Workspace and imports are done with ~:

// /packages/core/components/alert/alert.scss

@import "~@dso-toolkit/sources/src/components/alert/alert";

The ~ operator takes care of module resolving. /packages/sources/package.json contains name: "@dso-toolkit/sources.

johnjenkins commented 2 years ago

OIC! That's more of an issue with mono-repos in general I think - something I'm also working on :)

tfrijsewijk commented 2 years ago

I just found out the problem also exists when importing with relative paths:

// before
@import "~@dso-toolkit/sources/src/components/alert/alert";

// after
@import "../../../sources/src/components/alert/alert";

I don't think it has anything to do with Yarn Workspaces or mono repos πŸ€” It's as if Stencil figures out that a file is outside of the project and caches it really hard?

rwaskiewicz commented 2 years ago

πŸ‘‹

I wasn't able to reproduce the originally stated issue locally with the following stencil.config.ts changes from the output of npm init stencil component:

diff --git a/stencil.config.ts b/stencil.config.ts
index 141c3d3..871126a 100644
--- a/stencil.config.ts
+++ b/stencil.config.ts
@@ -1,7 +1,11 @@
 import { Config } from '@stencil/core';
+import { sass } from '@stencil/sass';

 export const config: Config = {
   namespace: 'nest',
+  plugins: [
+    sass()
+  ],
   outputTargets: [
     {
       type: 'dist',

https://user-images.githubusercontent.com/1930213/187930210-2f5f7a77-ad71-4a00-8a73-d457560863cf.mov

@tfrijsewijk can you provide me with a minimal reproduction case to take a closer look?

tfrijsewijk commented 2 years ago

@rwaskiewicz πŸ‘‹

I'll see what I can do, but I think you're pretty close to my reproduction case already:

It's important to note that style2.scss lives outside of the Stencil app directory.

jared-christensen commented 1 year ago

I want to clarify that there are two distinct issues that seem related, but I believe only one of them has been the focus of this discussion.

The original issue pertains to using a SASS import in a component SCSS file. In the past, changing the contents of the imported SCSS file was not reflected in the browser. However, this issue appears to have been resolved as it's working for me in my project using Stencil 3. For more information, refer to https://github.com/ionic-team/stencil-sass/issues/29.

A related, but different issue that I've noticed is when using SCSS as a globalStyle in the Stencil config (e.g., globalStyle: 'src/index.scss'). If you import another file in index.scss, changes to the imported file are not built unless you resave the index.scss. To reproduce this issue, you can start the Stencil server in watch mode, add a SASS import to your globalStyle file, and then make a change to the imported file. Upon checking the browser, you will find that your changes have not taken effect. As of today, this issue seems to persist.

I believe this second issue, while related, is distinct from the original topic of this thread. It looks like the second issue is being worked on here https://github.com/ionic-team/stencil/pull/3110

houh60 commented 1 year ago

As a work around:

  rollupPlugins: {
    before: [
      {
        name: 'watch-external',
        async buildStart() {
          const styleFiles = await fg(resolve(__dirname, './components/**/*.scss'));
          for (let file of styleFiles) {
            this.addWatchFile(file);
          }
        },
      },
    ],
  },

Note: I'm using the node fs resolve function and a library called fast-glob (aka fg). The code snippet is added to the stencil.config.js

Still, it doesn't work.😣

houh60 commented 1 year ago

The workaround mentioned in #2635 (comment) doesn't seem to work for me. But I'm not using the latest Stencil version. Can anyone confirm if this is an existing issue in the latest Stencil version?

I think I'm using the latest version of Stencil. But it's still the problem -- it doesn't pick up changes in imported style files.

James-Wilkinson-git commented 1 year ago

I just found out the problem also exists when importing with relative paths:

// before
@import "~@dso-toolkit/sources/src/components/alert/alert";

// after
@import "../../../sources/src/components/alert/alert";

I don't think it has anything to do with Yarn Workspaces or mono repos πŸ€” It's as if Stencil figures out that a file is outside of the project and caches it really hard?

I have this same issue, we recently moved our styles to a global scoped folder so they can be shared with our design system, stencil can read them on build, but can't watch them using relative paths that take you out of the stencil folder

christian-bromann commented 9 months ago

@James-Wilkinson-git @tfrijsewijk can someone provide a minimal reproducible example as I am unable to find any issues that are similar to what you've described. It would help me greatly to further investigate this. Maybe just make a PR to this repo: https://github.com/christian-bromann/stencil-style-repro

James-Wilkinson-git commented 9 months ago

Hi @christian-bromann here you go https://github.com/James-Wilkinson-git/stencil-bug

To Reproduce, clone, cd into stencil-install/stencil, and install then start the server npm run start then go to shared-style, and edit the scss file, you will see live reload does not work. In order to see your changes you need to shut down the server and restart it to get it to compile your new changes.

This is an issue because of using stencil in a mono-repo setting, where the shared-styles are used in more than one place not just in the stencil component, such as if someone was to use just HTML of our stencil template they could import the styles, or when creating a global stylesheet to include where we are not using stencil.

christian-bromann commented 9 months ago

Thanks for providing the reproducible example. I was able to reproduce the problem and will move this into our backlog.

silvertech-daniel commented 9 months ago

Our solution has been to use a script that watches all of the scss files and then triggers an update to the "globalStyle" file:

import { utimes, watch } from 'fs/promises';

const debounce = (f, ms = 100) => {
    let timeout;

    return (...args) => {
        if (timeout !== undefined)
            clearTimeout(timeout);

        timeout =
            setTimeout(() => {
                timeout = undefined;
                f(...args);
            }, ms);
    };
};

const src = new URL('./src/', import.meta.url);
const globalSassFile = new URL('./global/_global.scss', src);

const triggerGlobalSassUpdate = debounce(async () => {
    const fakeModifiedTime = new Date();
    await utimes(globalSassFile, fakeModifiedTime, fakeModifiedTime);
    console.log('triggered global sass update');
});

for await (const event of watch(src, { recursive: true })) {
    if (!event.filename)
        continue;

    const fileUrl = new URL(event.filename, src);
    if (
        fileUrl.pathname.endsWith('.scss') &&
        fileUrl.pathname !== globalSassFile.pathname
    )
        triggerGlobalSassUpdate();
}

However, this workaround stopped working in Stencil 4. Last time I tried, it did not work even if I manually edited a component's "styleUrl" file or the project's "globalStyle" file directly. The solution (slow) is to restart the build completely.

christian-bromann commented 9 months ago

@silvertech-daniel thanks for the feedback, I am currently revisiting a set of similar bugs and experienced similar behavior. We will ingest these issues into our backlog and address them accordingly now that we have validated the bugs and have a better idea about the impact on the user.

christian-bromann commented 9 months ago

The fix for this issue has been released as a part of today's Stencil v4.11.0 release.

James-Wilkinson-git commented 8 months ago

I;ve updated to 4.12.2 and the issue still remains. In our main project the watcher detects the changes but dosen't recompile the css, in the supplied bug protect, the watcher doesnt even detect the changes, was there a regression in 4.12 from 4.11?

James-Wilkinson-git commented 8 months ago

Installed 4.11.0 and confirmed that the watcher was still not picking up the scss from outside the project

christian-bromann commented 8 months ago

@James-Wilkinson-git mhm πŸ€” I wonder how your project is set up. Mind providing a minimal reproducible example? I would be happy to take a look.

James-Wilkinson-git commented 8 months ago

@christian-bromann https://github.com/James-Wilkinson-git/stencil-bug I upgraded this to the latest and it was not working.

silvertech-daniel commented 8 months ago

@James-Wilkinson-git update @stencil/sass, it should be at least 3.0.9

https://github.com/ionic-team/stencil/issues/5287#issuecomment-1915019923

James-Wilkinson-git commented 7 months ago

I've run into this problem again. Stencil v4.13.0 and @stencil/sass v3.0.11 repo updated https://github.com/James-Wilkinson-git/stencil-bug/blob/main/stencil-install/stencil/package-lock.json

rwaskiewicz commented 7 months ago

@James-Wilkinson-git can you please open a new issue?