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

feature/issue 1233 CSS Modules ™️ plugin #1285

Merged
merged 14 commits into from
Oct 18, 2024

Conversation

thescientist13
Copy link
Member

@thescientist13 thescientist13 commented Oct 7, 2024

Related Issue

resolves #1233

Summary of Changes

  1. Create a CSS Modules ™️ (mostly spec compliant) plugin
  2. Add to list of plugins on the website
  3. Added some narrowing to Rollup bundling to only exclude JS assets from using Greenwood plugins for resolving

TODO

  1. handle module shims hardcoding check
  2. fix windows test cases
  3. confirm if response.clone() / mergeResponse in prerender lifecycle is actually needed - yes, and we should include it anyway
  4. why do we need to return a CSS module via HTTP in the plugin? - ah, yeah I think we need this prerendering support
  5. logging / comments / misc refactoring + cleanup
  6. website repo validation - chore/upgrade greenwood v0.30.0 alpha.7 www.greenwoodjs.dev#107
  7. I think graph.js change should go over to rfc/issue 1167 content as data #1266 - https://github.com/ProjectEvergreen/greenwood/pull/1266/files#r1792691572
  8. we're only recursively handling .js files, what about TS, etc? - feature/issue 1233 CSS Modules ™️ plugin #1285 (comment)

Nice to have

  1. better implementation than having to write to a file on disk (cssModulesMap.json) - the main issue here is that for SSR / prerender, we can't save to globalThis since memory is not shared in Worker threads, so will have to leave it for now
  2. warn (or eliminate) unused / undefined styles (e.g. selector from the CSS is not found in the HTML). maybe with option to fail on build? otherwise by default this will just "break" the client, which is good / better I guess?
    Screenshot 2024-10-07 at 6 07 49 PM
  3. allow additional import identifier
    /* works ✅ */
    import styles from './header.module.css';
    
    /* does not work 🚫 */
    import header from './header.module.css';
  4. will this handle recursive references to components called using import (or only top level <script> tags) - seems to be working just fine per test cases verification

@thescientist13 thescientist13 added documentation Greenwood specific docs Plugins Greenwood Plugins CLI feature New feature or request labels Oct 7, 2024
@thescientist13 thescientist13 linked an issue Oct 8, 2024 that may be closed by this pull request
@thescientist13 thescientist13 marked this pull request as ready for review October 9, 2024 13:04
@thescientist13
Copy link
Member Author

So the topic of custom formats, we are principally limited to what we can reasonably cover with acorn due to the fact that, at least as of now, our implementation here depends on us crawling through all <script> tags in the HTML and extracting out all the styles, and needing to do that recursively.

Unfortunately, acorn on supports walking synchronously, which is in conflict with processing all these files first using Greenwood plugins (like say we do in rollup.config.js) since plugins are async.

So for now, just doing what I did in WCC in adding basic support for TypeScript stripping and called out whats supported in the plugin's README.

Perhaps there is a better implementation to be found, so definitely open to PRs that could help work around this problem, but i think this is good enough for now. 😇

@thescientist13 thescientist13 merged commit 03f5999 into release/0.30.0 Oct 18, 2024
8 checks passed
@thescientist13 thescientist13 deleted the feature/issue-1233-css-modules branch October 18, 2024 21:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLI documentation Greenwood specific docs feature New feature or request Plugins Greenwood Plugins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CSS Modules ™️ plugin
1 participant