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.

Sign up for GitHub

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

Build CJS, update package.json #701

Closed
wants to merge 6 commits into from
Closed

Build CJS, update package.json #701

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
7e52522
add exports field to package.json
bschlenk Mar 27, 2024
a104ab2
Output CommonJS files as .cjs with Rollup
KittyGiraudel Aug 14, 2024
4f71fba
Use .cjs files in Cypress fixtures
KittyGiraudel Aug 14, 2024
644f9b8
Update the exports configuration in package.json
KittyGiraudel Aug 14, 2024
8eb7233
Merge branch 'main' into main
KittyGiraudel Aug 14, 2024
4228ad3
Build CJS, update package.json exports
drwpow-figma Aug 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,15 @@
"main": "dist/a11y-dialog.js",
"module": "dist/a11y-dialog.esm.js",
"types": "dist/a11y-dialog.d.ts",
"exports": {
".": {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed elsewhere, I guess the "." character is optional; I’m just used to seeing this syntax more. Probably doesn’t matter.

"import": "./dist/a11y-dialog.esm.js",
"browser": "./dist/a11y-dialog.js",
"require": "./dist/a11y-dialog.cjs",
"types": "./dist/a11y-dialog.d.ts"
},
"./*": "./*"
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like doing this: just exposing all files by default, as-typed. This is safer than having to manually type out every file one-by-one (and newer files don’t get exposed if they’re not typed out).

IMO trying to be clever, and trying to anticipate how people have manually-imported specific files from /dist/, can backfire unless you’ve tested every possible setup. Instead, I like to trust that users do the right thing, and if they’re manually importing a file, just let them.

},
"keywords": ["modal", "dialog", "accessibility", "a11y", "focus"],
"author": "Kitty Giraudel (https://kittygiraudel.com)",
"repository": {
Expand Down
12 changes: 11 additions & 1 deletion rollup.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { createRequire } from 'node:module'
import terser from '@rollup/plugin-terser'
import { nodeResolve } from '@rollup/plugin-node-resolve'
import terser from '@rollup/plugin-terser'
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This got formatted by the githook IDK

import typescript from '@rollup/plugin-typescript'

const require = createRequire(import.meta.url)
Expand Down Expand Up @@ -55,6 +55,16 @@ export default [
},
],
},
{
input: 'src/index.ts',
plugins: plugins,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: I believe it’s generating the correct thing using the existing tsconfig.json. But best practice is to make a 2nd TS Config (e.g. tsconfig.cjs.json) if it stops working as-expected (worth noting that the current TSConfig uses legacy module resolution, so you may have to split it up if you used "module": "ESNext", "moduleResolution": "Bundler": which is now the recommended setup for most frontend code.

output: [
{
file: 'dist/a11y-dialog.cjs',
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New file for Node.js that doesn’t change contents of previous files.

format: 'cjs',
},
],
},
{
input: 'src/dom-utils.ts',
plugins: plugins,
Expand Down