Add style export condition for stylesheet - #54
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the package’s conditional exports to better support consuming the published ./styles.css entry via tooling that recognizes style and/or relies on a default fallback condition.
Changes:
- Adds a
"style"export condition for./styles.csspointing at the built stylesheet. - Adds a
"default"export condition for./styles.cssas a fallback alongside existing"import"/"require"mappings.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![constructor-claude-bedrock[bot]](https://avatars.githubusercontent.com/u/9828194?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This PR adds style and default export conditions for ./styles.css in package.json. The style condition is a useful addition for bundlers like Vite and webpack, but the default condition is incorrectly configured and the require condition may be silently bypassed.
Inline comments: 2 discussions added
Overall Assessment:
| "import": "./lib/mjs/styles.css", | ||
| "require": "./lib/cjs/styles.css" | ||
| "require": "./lib/cjs/styles.css", | ||
| "default": "./lib/mjs/styles.css" |
There was a problem hiding this comment.
Important Issue: The "default" condition points to the MJS version, but "default" is a catch-all fallback that matches any environment not already matched by a prior condition. Because CSS files are module-system agnostic, CommonJS consumers that happen to hit "default" (e.g., bundlers that don't explicitly activate the require condition) will receive ./lib/mjs/styles.css instead of the intended ./lib/cjs/styles.css.
If a separate CJS stylesheet is needed, "default" should point to the CJS path for maximum compatibility:
"default": "./lib/cjs/styles.css"Alternatively, if both CSS files are identical in content (very common for stylesheets), a single shared file can be used for all three conditions (style, import/require, and default), eliminating the ambiguity entirely.
| "types": "lib/types/index.d.ts", | ||
| "exports": { | ||
| "./styles.css": { | ||
| "style": "./lib/mjs/styles.css", |
There was a problem hiding this comment.
Suggestion: The "style" condition is a non-standard, community convention recognised by bundlers such as Vite, webpack, and Parcel to identify CSS entry points. It is a welcome addition. However, since CSS files carry no ESM/CJS semantics, consider whether having two separate build artefacts (lib/mjs/styles.css and lib/cjs/styles.css) is actually necessary. If the compiled output is identical, a single lib/styles.css referenced by all conditions would simplify the copy-styles build script and remove the need for the "default" condition ambiguity discussed above.
Adds style and default export conditions for ./styles.css.