-
Notifications
You must be signed in to change notification settings - Fork 492
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
Use package.json#exports
#1185
Use package.json#exports
#1185
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
--- | ||
"@changesets/get-version-range-type": minor | ||
"@changesets/assemble-release-plan": minor | ||
"@changesets/get-dependents-graph": minor | ||
"@changesets/apply-release-plan": minor | ||
"@changesets/changelog-github": minor | ||
"@changesets/get-release-plan": minor | ||
"@changesets/get-github-info": minor | ||
"@changesets/changelog-git": minor | ||
"@changesets/release-utils": minor | ||
"@changesets/test-utils": minor | ||
"@changesets/config": minor | ||
"@changesets/errors": minor | ||
"@changesets/logger": minor | ||
"@changesets/parse": minor | ||
"@changesets/types": minor | ||
"@changesets/write": minor | ||
"@changesets/read": minor | ||
"@changesets/cli": minor | ||
"@changesets/git": minor | ||
"@changesets/pre": minor | ||
--- | ||
|
||
`package.json#exports` have been added to limit what (and how) code might be imported from the package. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,8 +2,20 @@ | |
"name": "@changesets/apply-release-plan", | ||
"version": "6.1.4", | ||
"description": "Takes a release plan and applies it to packages", | ||
"main": "dist/apply-release-plan.cjs.js", | ||
"module": "dist/apply-release-plan.esm.js", | ||
"main": "dist/changesets-apply-release-plan.cjs.js", | ||
"module": "dist/changesets-apply-release-plan.esm.js", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this field will have no effect. it will be ignored in favor of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's here to support older bundlers that don't support There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fair, I guess. Though it seems hard to imagine that people are both bundling changeset packages to create their own changeset tools and doing so with a bundler that's that old There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree but this is auto-generated by a somewhat universal library builder ( https://github.com/preconstruct/preconstruct/ ) and for a lot of npm packages it might still make sense to have proper compat with webpack 4 etc. |
||
"exports": { | ||
".": { | ||
"types": { | ||
"import": "./dist/changesets-apply-release-plan.cjs.mjs", | ||
"default": "./dist/changesets-apply-release-plan.cjs.js" | ||
}, | ||
"module": "./dist/changesets-apply-release-plan.esm.js", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this is totally non-standard. is it really necessary? it would be much nicer not to have it There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It is totally standard :p it's just not described by node - the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, thank you for educating me! I had no idea Rollup supported this, but it looks like it's been in |
||
"import": "./dist/changesets-apply-release-plan.cjs.mjs", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean this in the nicest possible way, but it is absolutely crazy to use There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is an MJS wrapper - it's an ESM file that reexports CJS file. I know it looks funny but it kinda makes perfect sense to us. It's also kinda irrelevant rly - the extension is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's helpful to know. While I agree it's irrelevant to consumers of the package, it isn't necessarily intuitive to people who might want to contribute to it. Something like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Well, usually you just don't touch those files at all. When contributing you just interact directly with sources and all of this here is auto-generated for us. I appreciate the feedback but at the end of the day, I consider this particular thing a very minor detail that is kinda irrelevant. I can see how it might be surprising at first glance though. |
||
"default": "./dist/changesets-apply-release-plan.cjs.js" | ||
}, | ||
"./package.json": "./package.json" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm afraid we started this convention because of some ill-behaved versions of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's to support |
||
}, | ||
"license": "MIT", | ||
"repository": "https://github.com/changesets/changesets/tree/main/packages/apply-release-plan", | ||
"dependencies": { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,4 @@ | ||
/* | ||
BAD CODE ALERT! | ||
|
||
You should never reach out of one package and into another in a multi-package repository. | ||
(doing so is a leading cause of 'works on my machine' but then failure when the packages are published) | ||
|
||
We are doing it here to avoide adding a circular dependency and as this is only used in testing. | ||
|
||
This is wicked, and please don't copy us. | ||
*/ | ||
|
||
export { default } from "../../../cli/changelog"; | ||
// We are doing it here to avoide adding a circular dependency and as this is only used in testing. | ||
// This is wicked, and please don't copy us. | ||
// eslint-disable-next-line import/no-extraneous-dependencies | ||
export { default } from "@changesets/cli/changelog"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,4 @@ | ||
/* | ||
BAD CODE ALERT! | ||
|
||
You should never reach out of one package and into another in a multi-package repository. | ||
(doing so is a leading cause of 'works on my machine' but then failure when the packages are published) | ||
|
||
We are doing it here to avoide adding a circular dependency and as this is only used in testing. | ||
|
||
This is wicked, and please don't copy us. | ||
*/ | ||
|
||
export { default } from "../../../cli/commit"; | ||
// We are doing it here to avoide adding a circular dependency and as this is only used in testing. | ||
// This is wicked, and please don't copy us. | ||
// eslint-disable-next-line import/no-extraneous-dependencies | ||
export { default } from "@changesets/cli/commit"; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,4 @@ | ||
{ | ||
"main": "dist/cli.cjs.js", | ||
"module": "dist/cli.esm.js", | ||
"preconstruct": { | ||
"source": "../src/changelog" | ||
} | ||
"main": "dist/changesets-cli-changelog.cjs.js", | ||
"module": "dist/changesets-cli-changelog.esm.js" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,4 @@ | ||
{ | ||
"main": "dist/cli.cjs.js", | ||
"module": "dist/cli.esm.js", | ||
"preconstruct": { | ||
"source": "../src/commit" | ||
} | ||
"main": "dist/changesets-cli-commit.cjs.js", | ||
"module": "dist/changesets-cli-commit.esm.js" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some of those are in the
0.x
range so I could consider usingpatch
for them - it doesn't quite matter for them though anyway so I'm just usingminor
for every pkg here