Build: Bundle lib/api with ts-up #19269
Conversation
|
@shilman I think this works and is ready for merge! |
| @@ -1,5 +1,7 @@ | |||
| // eslint-disable-next-line @typescript-eslint/triple-slash-reference | |||
| /// <reference path="./typings.d.ts" /> | |||
| /// <reference types="node" /> | |||
| /// <reference types="webpack-env" /> | |||
| @@ -51,6 +51,9 @@ | |||
| "@storybook/theming": "7.0.0-alpha.34", | |||
| "global": "^4.4.0" | |||
| }, | |||
| "devDependencies": { | |||
| "@types/webpack-env": "^1.16.0" | |||
There was a problem hiding this comment.
I think there's a bug with NX where it builds dependents without the --optimized flag
That might be the cause of a few issues... one of those issues I got fixed by adding this, but it's not really what i want to do..
I've reached out to Katarina already
There was a problem hiding this comment.
Can you explain a little more why that leads to needing to add this? I'm not really following..
| @@ -51,6 +51,9 @@ | |||
| "@storybook/theming": "7.0.0-alpha.34", | |||
| "global": "^4.4.0" | |||
| }, | |||
| "devDependencies": { | |||
| "@types/webpack-env": "^1.16.0" | |||
There was a problem hiding this comment.
Can you explain a little more why that leads to needing to add this? I'm not really following..
| "./shortcut": { | ||
| "require": "./dist/shortcut.js", | ||
| "import": "./dist/shortcut.mjs", | ||
| "types": "./dist/shortcut.d.ts" | ||
| }, |
| @@ -66,6 +66,8 @@ | |||
| "@storybook/semver": "^7.3.2", | |||
| "@storybook/theming": "7.0.0-alpha.34", | |||
| "@testing-library/react": "^11.2.2", | |||
| "@types/node": "^14.0.10 || ^16.0.0", | |||
| "@types/webpack-env": "^1.16.0", | |||
There was a problem hiding this comment.
Don't we build these with esbuild, not webpack? Is it weird to depend on webpack env types?
There was a problem hiding this comment.
module.hot.accept does not exist in node, that's a webpack runtime thing
| @@ -66,6 +66,8 @@ | |||
| "@storybook/semver": "^7.3.2", | |||
| "@storybook/theming": "7.0.0-alpha.34", | |||
| "@testing-library/react": "^11.2.2", | |||
| "@types/node": "^14.0.10 || ^16.0.0", | |||
There was a problem hiding this comment.
This doesn't run in the node context, right? What node types are we using?
| ".": { | ||
| "require": "./dist/index.js", | ||
| "import": "./dist/index.mjs", | ||
| "types": "./dist/index.d.ts" |
There was a problem hiding this comment.
FYI, this export map is incorrect - the types condition will never be matched, since import or require will always be matched first (so we'll always be looking at the default location for the declaration file - adjacent to the source file). Moreover, you very likely want a dedicated declaration file for each of the .js and .mjs entrypoints, so TS knows the correct module format of the import for analysis. So you want something more like
"exports": {
".": {
"require": { "types": "./dist/index.d.ts", "default": "./dist/index.js" },
"import": { "types": "./dist/index.d.mts", "default": "./dist/index.mjs" }
},
"./shortcut": {
"require": { "types": "./dist/shortcut.d.ts", "default": "./dist/shortcut.js" },
"import": { "types": "./dist/shortcut.d.mts", "default": "./dist/shortcut.mjs" }
},
"./package.json": "./package.json"
},Naturally, you'll need those .d.mts files to actually exist.
There was a problem hiding this comment.
Thank you for this comment!
Moreover, you very likely want a dedicated declaration file for each of the .js and .mjs entrypoints
Can you explain why? what would be the difference in content of these 2 files?
What does typescript use the "correct module format of the import for analysis" for?
so we'll always be looking at the default location for the declaration file - adjacent to the source file
So this is why it works out in the end, right?
Could we simply remove the "types" line, then?
There was a problem hiding this comment.
Can you explain why? what would be the difference in content of these 2 files?
The .d.ts file will be recognized as representing a common js module, while the .d.mts file will be recognized as representing a ecmascript module, which have a complex non-transparent interop scheme in node. Notably, it's an error to load esm from a common js module via require (and this can't be papered over with an import helper), so detecting this in advance requires source-accurate declaration file module format indicators.
The index.d.mts file can likely be export * from "./index.js"; if the named exports are meant to be similar. If there's meant to be a default member, you may need that, too.
What does typescript use the "correct module format of the import for analysis" for?
Knowing if the default import from an es module should be the whole module (because the target is cjs), knowing if the import should be an error (because the target is esm and the require'ing module is cjs).
There was a problem hiding this comment.
Thank you so much for taking the time to explain of of this to me (and others reading)!
Would you be able to assist us with this migration?
related: #18732