Conversation
🦋 Changeset detectedLatest commit: 3bf46a7 The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 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 |
packages/remix-node/rollup.config.js
Outdated
| { | ||
| external: (id) => isBareModuleId(id), | ||
| input: `${SOURCE_DIR}/index.ts`, | ||
| input: [`${SOURCE_DIR}/index.ts`, `${SOURCE_DIR}/install.ts`], |
There was a problem hiding this comment.
Now that we have the exports field, we can point to the dist folder and remove the hard-coded install.js and install.d.ts files.
| input: `${SOURCE_DIR}/index.ts`, | ||
| output: { | ||
| file: `${OUTPUT_DIR}/index.js`, | ||
| file: `${OUTPUT_DIR}/index.mjs`, |
There was a problem hiding this comment.
We have to use the mjs extension now to prevent this file being resolved as CJS via the import field within exports.
| /* eslint-disable */ | ||
| "use strict"; | ||
|
|
||
| var globals = require("./dist/globals.js"); | ||
|
|
||
| Object.defineProperty(exports, "__esModule", { value: true }); | ||
|
|
||
| globals.installGlobals(); |
There was a problem hiding this comment.
As mentioned, this file and its types are now built into the dist folder.
| "exports": { | ||
| "./package.json": "./package.json" | ||
| }, |
There was a problem hiding this comment.
This is only being added to prevent reaching into package internals.
brophdawg11
left a comment
There was a problem hiding this comment.
LGTM but I would probably get a 👍 from @jacob-ebey as well since he knows the nuances of this stuff really well
|
@mjackson @jacob-ebey and I chatted yesterday about the ESM/CJS stuff and came up with a gameplan that we think should work for v7 - filed that as a separate task in https://github.com/orgs/remix-run/projects/21/views/1 so I think this can probably get merged and then w'ell tackle that task and we can start experimenting with deployed builds on the nightlies? |
|
I might add a major changeset too just so we have something to catch our eye and elaborate on it a bit in the release notes too as something for folks to be aware of if they hit any bundler issues. |
| "exports": { | ||
| ".": { | ||
| "types": "./dist/index.d.ts", | ||
| "import": "./dist/index.mjs", |
There was a problem hiding this comment.
I had to add an ESM build to fix a dual-package-hazard issue. In the upload-test integration test we have an instanceof MaxPartSizeExceededError check that started failing because the app and @react-router/node imported different builds of React Router. It seems the move to using exports triggered Playwright to load the ESM build rather than the CJS build.
|
🤖 Hello there, We just published version Thanks! |
The
exportsfield was added toreact-routerandreact-router-domas part of #11669, but this PR standardises this across all packages. I've split this out into a separate PR as a standalone decision.