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: Barrel files which re-export multiple Astro components are eagerly evaluated #701

Closed
natemoo-re opened this issue Jul 15, 2021 · 8 comments
Labels
feat: styling Related to styles (scope)
Milestone

Comments

@natemoo-re
Copy link
Member

What package manager are you using?

npm

What operating system are you using?

Mac

Describe the Bug

The Snowpack plugin which powers Astro uses the load hook to compile .astro files. Given a file that re-exports many Astro files:

export { default as Markdown } from './Markdown.astro';
export { default as Prism } from './Prism.astro';
export { default as Debug } from './Debug.astro';

If any of the exported Astro components have a <style> tag, that <style> will be injected whether the component is used or not.

This is because Astro's compilation is stateful. Every .astro file bubbles up <style> tags to the root .astro page. In this case, that's not desired and these components should be evaluated lazily.

See #675 (comment)

Steps to Reproduce

  1. npm init astro
  2. create components/index.js that exports two .astro components, A and B.
  3. in an .astro page, use import { A } from '../components/index.js.
  4. styles from B are injected onto page, despite B not being rendered.

Link to Minimal Reproducible Example (Optional)

No response

@FredKSchott
Copy link
Member

I think this is intentional, based on how Snowpack, Vite, webpack, etc. all work. I'm not sure if it's so much about state as much as its about the behavior: "if we see a CSS import, we inject it as a style on the page".

IMO this feels okay to me for development, but not build. When we build, we tree-shake the dependencies, so we should already be treeshakeing the unused CSS out automatically. If not, maybe that's the issue? @natemoo-re can you confirm that this is only impacting dev?

@FredKSchott
Copy link
Member

I remember leaving a comment about the builtin debug component shouldn't be able to add global rules to the page. Maybe we enforce a rule of "no astro/component can add global CSS rules" to make sure the dev experience isn't impacted by this?

@jasikpark
Copy link
Contributor

Well, wouldn't this be affected by other packages that re-export components like https://docs.astro.build/guides/publish-to-npm#indexjs recommends?

@FredKSchott
Copy link
Member

That's correct. This affects all build tools that allow you to `import './some/file.css' and tree-shaking in prod is the usual answer.

Happy to come up with something better! just wanted to make sure that that was understood

@matthewp
Copy link
Contributor

@natemoo-re does the solution @FredKSchott outlined work for you?

@natemoo-re
Copy link
Member Author

@natemoo-re does the solution @FredKSchott outlined work for you?

It works for now. With tree-shaking or compiler improvements we might be able to tackle this in the future.

What prompted this was the Debug component, but maybe we could inject the debug styles separately in dev?

@jasikpark
Copy link
Contributor

Would it work to only lazily export the Debug component?

@drwpow drwpow added feat: styling Related to styles (scope) compiler-v2 labels Aug 19, 2021
@natemoo-re
Copy link
Member Author

Going to close this as part of astro@next. Imports are still crawled and SSR'd, but styles won't be injected like the currently are in Astro Classic™. Styles are only injected when components are rendered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat: styling Related to styles (scope)
Projects
None yet
Development

No branches or pull requests

5 participants