Uh oh!
There was an error while loading. Please reload this page.
feat: only publish esm bundles from @primer/react - #6425
Conversation
🦋 Changeset detectedLatest commit: 8824fb9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 2 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
👋 Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks! |
size-limit report 📦
|
There was a problem hiding this comment.
Pull Request Overview
This PR converts @primer/react to be ESM-only by removing CommonJS and UMD builds and updating the package configuration to use modern ESM conventions.
- Simplifies build output to only ESM bundles in the
distdirectory - Updates package.json exports configuration with proper types and default conditions
- Modernizes scripts and configurations to work in an ESM context
Reviewed Changes
Copilot reviewed 11 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/react/package.json | Updates package type to "module", consolidates exports to single dist directory, adds publint for package validation |
| packages/react/rollup.config.mjs | Removes CommonJS and UMD build configurations, keeps only ESM output |
| packages/react/tsconfig.json | Changes TypeScript output directory from lib to dist |
| packages/react/script/build | Removes CommonJS-related build steps and file copying |
| packages/react/script/precompile-color-schemes.ts | Adds ESM __dirname polyfill using import.meta.url |
| packages/react/script/components-json/build.ts | Updates Babel imports to use named exports for ESM compatibility |
| packages/react/tsconfig.build.json | Expands test file exclusions to be more specific |
| packages/react/babel.config.cjs | Updates require path to use .cjs extension |
| .github/workflows/ci.yml | Adds publint validation step and updates script reference |
| .github/workflows/size.yml | Removes size limit workflow |
| .changeset/slick-months-cry.md | Adds changeset for major version bump |
| "import": "./lib-esm/test-helpers.js", | ||
| "require": "./lib/test-helpers.js" | ||
| "types": "./dist/utils/test-helpers.d.ts", | ||
| "default": "./dist/test-helpers.js" |
There was a problem hiding this comment.
There's a mismatch between the types path (./dist/utils/test-helpers.d.ts) and the default path (./dist/test-helpers.js). Both should point to the same base file location - either both should be in the root or both should be in the utils directory.
| "default": "./dist/test-helpers.js" | |
| "default": "./dist/utils/test-helpers.js" |
👋 Hi, there are new commits since the last successful integration test. We recommend running the integration workflow once more, unless you are sure the new changes do not affect github/github. Thanks! |
👋 Hi, there are new commits since the last successful integration test. We recommend running the integration workflow once more, unless you are sure the new changes do not affect github/github. Thanks! |
👋 Hi from github/github-ui! Your integration PR is ready: https://github.com/github/github-ui/pull/2540 |
🟢 ci completed with status |
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Marie Lucca <40550942+francinelucca@users.noreply.github.com>
Since primer#6425, the package only exports ESM modules. As a consequence, the rollup configuration does not need to export an array of configuration objects, and as yet another consequence, there is no need to define a base config that could be shared. Refactor rollup configuration to export a simple default object directly instead of an array.
Part of https://github.com/github/primer/issues/5541
Update
@primer/reactto be ESM-onlyChangelog
New
Changed
"type"field to be"module""exports"configuration so that:typesconditiondefaultcondition (which will not be ESM)lint:publishstep that checks that our package is setup correctly with its typesRemoved
Rollout strategy
We are considering this a major release since the package is no longer publishing ESM bundles. With recent releases of Node.js, we should not need to handle migration (since you can now import ESM from a CommonJS context) but we will test against dotcom to confirm.