-
-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
🐛 add exports in package.json #4048
base: main
Are you sure you want to change the base?
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 |
---|---|---|
@@ -1,8 +1,8 @@ | ||
{ | ||
"name": "styled-components/native", | ||
"private": true, | ||
"types": "./dist/native/index.d.ts", | ||
"main": "./dist/styled-components.native.cjs.js", | ||
"jsnext:main": "./dist/styled-components.native.esm.js", | ||
"module": "./dist/styled-components.native.esm.js" | ||
"types": "../dist/native/index.d.ts", | ||
"main": "../dist/styled-components.native.cjs.js", | ||
"jsnext:main": "../dist/styled-components.native.esm.js", | ||
"module": "../dist/styled-components.native.esm.js" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -10,10 +10,33 @@ | |
"./dist/styled-components.esm.js": "./dist/styled-components.browser.esm.js", | ||
"./dist/styled-components.cjs.js": "./dist/styled-components.browser.cjs.js" | ||
}, | ||
"exports": { | ||
".": { | ||
"types": "./dist/index.d.ts", | ||
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. Using 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. Yeah, I couldn't figure out how to emit mts/cts using rollup. Will need to dig into tsup's source and see how they did it |
||
"import": { | ||
"node": "./dist/styled-components.esm.js", | ||
"default": "./dist/styled-components.browser.esm.js" | ||
Comment on lines
+17
to
+18
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 order isn't exactly incorrect but I think it makes sense to think of node as the default and use browser as an override. Either way, you probably want to also include the |
||
}, | ||
"require": { | ||
"node": "./dist/styled-components.cjs.js", | ||
"default": "./dist/styled-components.browser.cjs.js" | ||
} | ||
Comment on lines
+16
to
+23
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. Those two share the same directory, they are meant to represent two different module formats (CJS vs ESM) and yet they are using the same extension ( |
||
}, | ||
"./macro": { | ||
"types": "./dist/index.d.ts", | ||
"import": "./dist/styled-components-macro.esm.js", | ||
"require": "./dist/styled-components-macro.cjs.js" | ||
}, | ||
"./native": { | ||
"types": "./dist/native/index.d.ts", | ||
"import": "./dist/styled-components.native.esm.js", | ||
"require": "./dist/styled-components.native.cjs.js" | ||
} | ||
}, | ||
"sideEffects": false, | ||
"scripts": { | ||
"generateErrors": "node scripts/generateErrorMap.js", | ||
"prebuild": "rimraf dist && yarn generateErrors", | ||
"prebuild": "rimraf dist native/dist && yarn generateErrors", | ||
"build": "rollup -c", | ||
"postbuild": "yarn size", | ||
"pretest": "yarn generateErrors", | ||
|
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.
You are attempting to ship so-called dual package here, thus you are making yourself vulnerable to the dual package hazard. Since styled-components rely on React context, I think that you should try to avoid it as much as possible. Using
.mjs
wrappers strategy would be a much better choice that would be safer for the users.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.
@Andarist I find this very interesting - do you know of any blogpost / resource where the
.mjs
wrappers strategy is explained it detail, also in regards to Typescript?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.
It's primarily described in node.js docs here but you can also find references to it in webpack and esbuild docs. We use this strategy in Emotion.
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.
ok, thanks. I read that, but got confused with regards to Typescript and the issues described above. I'll have a look on how Emotion handles this as reference for my own lib, thanks!