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

Sass watcher doesn't re-compile on changes to meta.load-css() loaded modules? #1808

Closed
mirisuzanne opened this issue Oct 2, 2022 · 4 comments · Fixed by #1877
Closed

Sass watcher doesn't re-compile on changes to meta.load-css() loaded modules? #1808

mirisuzanne opened this issue Oct 2, 2022 · 4 comments · Fixed by #1877
Assignees

Comments

@mirisuzanne
Copy link

mirisuzanne commented Oct 2, 2022

(I'm using v1.55.0)

In order to use CSS @layer efficiently on an existing code-base, I switched from e.g.:

/* screen.scss */
@use 'default';
@use 'layout';
@use 'theme';

to:

/* screen.scss */
@use 'sass:meta';

@layer default { @include meta.load-css('default'); }
@layer layout { @include meta.load-css('layout'); }
@layer theme { @include meta.load-css('theme'); }

That's by far the simplest upgrade path, rather than wrapping each module individually with a layer or mixin.

But since making that change, the watcher no longer compiles any changes to modules other than screen.scss itself. Changes to any of the loaded modules are ignored by the watcher. I don't see anything about that in the docs, or the specs, or any other issues mentioning it - but I was able to reproduce the problem consistently.

Is that the expected behavior? Are others able to reproduce this?

@nex3 nex3 self-assigned this Oct 3, 2022
@nex3
Copy link
Contributor

nex3 commented Oct 3, 2022

This is more or less expected. meta.load-css() is intended for dynamic loads, but one of the core issues with dynamic loads is that they aren't (in general) statically analyzable in the way we currently rely on to handle file watches.

We could potentially restructure the watcher logic to look at which files are actually loaded during a given compilation rather than relying on static analysis, but this would be a fairly substantial engineering effort. For now, I'll list this as a low-priority enhancement.

@nex3 nex3 transferred this issue from sass/sass Oct 3, 2022
@mirisuzanne
Copy link
Author

As CSS is adding more of these architectural tools (layers, scope, etc) - these issues are becoming more and more important. As we're designing these features at the CSSWG, the main question we always get is 'how can I load stylesheets into one of these structures'. In every case we end up adding additional @import/<link> tooling to make that possible. If Sass makes all those use cases more difficult, we become a major barrier to adoption.

I don't think that's a low priority to address somehow. It may be that we want to address it in some other way - but 'nested imports' are becoming more and more central to basic CSS functionality. We need to give that some attention.

@mirisuzanne
Copy link
Author

(sorry, I meant to open this in the sass repo - not sure why I ended up in dart-sass)

@nex3 nex3 removed the low priority label Oct 5, 2022
@nex3
Copy link
Contributor

nex3 commented Oct 5, 2022

Miriam and I talked about this, and we're going to handle it by special-casing @include meta.load-css() with a literal string argument as something the watcher can recognize.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants