Skip to content

Commit 8670b4d

Browse files
committed
docs: separate rationale
1 parent 666920c commit 8670b4d

File tree

4 files changed

+57
-37
lines changed

4 files changed

+57
-37
lines changed

README.md

+18-32
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,16 @@ Now you can publish your library with `npm run ubik publish`.
3333

3434
### Fixing extensions
3535

36-
The recommended way to do this is by publishing your package with:
36+
`tsc` outputs invalid JavaScript when building ESM libraries. [Read more](./docs/extensions.md)
37+
38+
The recommended way to fix this with Ubik is by using the following command
39+
instead of `npm publish` to publish your package to NPM:
3740

3841
```sh
3942
npm run ubik publish
4043
```
4144

42-
You can also do:
45+
You can also do this to apply the fix in place:
4346

4447
```sh
4548
npm run ubik compile
@@ -54,7 +57,12 @@ For best experience, add to `tsconfig.json`:
5457
{
5558
"exclude": [
5659
"dist/**/*"
57-
]
60+
],
61+
"compilerOptions": {
62+
"target": "esnext",
63+
"module": "esnext",
64+
"moduleResolution": "node10"
65+
}
5866
}
5967
```
6068

@@ -69,39 +77,12 @@ dist/
6977
*.dist.d.ts
7078
```
7179

72-
This assumes that the entrypoint of your package (`main` in `package.json`)
80+
**KNOWN ISSUE:** This operation assumes that the entrypoint of your package (`main` in `package.json`)
7381
is at the top level of your source code tree, e.g. `./index.ts` or `./src/index.ts` -
7482
but not e.g. `./src/foo/index.ts` next to `./src/bar/somethingelse.ts` (the latter
7583
would probably fail to compile all files or will place them in inappropriate locations -
7684
good matter for a pull request.)
7785

78-
#### Fixing extensions - rationale
79-
80-
TypeScript wants you to import modules with:
81-
82-
```ts
83-
import { foo } from "./foo"
84-
```
85-
86-
but the ECMAScript specification expects:
87-
88-
```js
89-
import { foo } from "./foo.js"
90-
```
91-
92-
TypeScript doesn't add the extensions; it also doesn't provide an extensibility hook
93-
which could be used to add them. [The documentation](https://www.typescriptlang.org/docs/handbook/esm-node.html)
94-
is confusing and unhelpful; the solution that it suggests is ridiculous and unacceptable.
95-
On GitHub issues, the TypeScript developers have repeatedly reacted with indifference.
96-
97-
**Before Node.js 16, this was less of a problem:** imports without extensions still
98-
worked. However, since Node 16 started enforcing the ES Modules spec (which requires
99-
extensions), the code that `tsc` outputs became effectively **invalid and impossible to run**
100-
(unless you were writing your program in a single file).
101-
102-
What, were you expecting the compiler for evertone's favorite "strict superset of JS"
103-
to, idk, output correct JS that is ready to execute? You crazy person!
104-
10586
### Splitting away type imports
10687

10788
```sh
@@ -110,8 +91,13 @@ to, idk, output correct JS that is ready to execute? You crazy person!
11091

11192
### Fixing CommonJS star imports
11293

94+
When targeting ESM on Node, CommonJS imports are wrapped in an extra `default` key,
95+
of which TypeScript is unaware. [Read more](./docs/split-stars.md)
96+
97+
Let's rewrite the imports so that both work:
98+
11399
```sh
114-
// TODO
100+
npm exec ubik split-stars ./src -- protobufjs
115101
```
116102

117103
### Others

docs/extensions.md

+24
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
TypeScript wants you to import modules with:
2+
3+
```ts
4+
import { foo } from "./foo"
5+
```
6+
7+
but the ECMAScript specification expects:
8+
9+
```js
10+
import { foo } from "./foo.js"
11+
```
12+
13+
TypeScript doesn't add the extensions; it also doesn't provide an extensibility hook
14+
which could be used to add them. [The documentation](https://www.typescriptlang.org/docs/handbook/esm-node.html)
15+
is confusing and unhelpful; the solution that it suggests is ridiculous and unacceptable.
16+
On GitHub issues, the TypeScript developers have repeatedly reacted with indifference.
17+
18+
**Before Node.js 16, this was less of a problem:** imports without extensions still
19+
worked. However, since Node 16 started enforcing the ES Modules spec (which requires
20+
extensions), the code that `tsc` outputs became effectively **invalid and impossible to run**
21+
(unless you were writing your program in a single file).
22+
23+
What, were you expecting the compiler for evertone's favorite "strict superset of JS"
24+
to, idk, output correct JS that is ready to execute? You crazy person!

docs/split-stars.md

+10
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
This compatibility issue was discovered when trying to port a Protobuf.js-based API client
2+
to import maps.
3+
4+
As of 2023-09-27, Protobuf is a CommonJS-based library. Polyfilling it in the
5+
browser is perfectly doable. However, the generated protobuf bindings (what I assume
6+
the files importing starting with `import * as _m0` to be) are not completely valid
7+
when targeting ESM on Node, because Node adds an extra `default` key around the library
8+
when importing CJS from ESM, of which TypeScript is blissfully unaware.
9+
10+
That ends up in a situation where you have either working code, or working types. Great!

ubik.cli.mjs

+5-5
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ try {
7474

7575
case 'split-stars': {
7676
if (argv.length === 0) {
77-
console.error('You did not provide any inputs.')
77+
console.error('You did not provide any arguments.')
7878
process.exit(1)
7979
}
8080
const split = argv.indexOf('--')
@@ -83,11 +83,11 @@ try {
8383
process.exit(1)
8484
}
8585
const pkgs = argv.slice(0, split)
86-
const srcs = argv.slice(split + 1)
8786
if (pkgs.length < 1) {
88-
console.error('You did not provide any packages imports of which to fix.')
87+
console.error('You did not provide any packages.')
8988
process.exit(1)
9089
}
90+
const srcs = argv.slice(split + 1)
9191
if (srcs.length < 1) {
9292
console.error('You did not provide any sources to process.')
9393
process.exit(1)
@@ -204,7 +204,7 @@ function printUsage () {
204204
console.info(' Provide more detailed descriptions of what each command does and why.')
205205
console.info(' ', bold('split-types'), '[--dry] [subdirs...]')
206206
console.info(` ${colors.yellow('Experimental.')} Fix types imported without "import type"`)
207-
console.info(' ', bold('split-stars'), '[--dry] packages... -- sources...')
207+
console.info(' ', bold('split-stars'), '[--dry] packagenames... -- sourcedirs...')
208208
console.info(` ${colors.yellow('Experimental.')} Fix default imports of CommonJS modules imported as ESM by Node`)
209209
console.info(' ', bold('fix-import-dirs'), '[--dry] [subdirs...]')
210210
console.info(` ${colors.yellow('Experimental.')} Add missing directory indices`)
@@ -242,7 +242,7 @@ function printRationale () {
242242
console.info(' separate "import type" or "export type ... from" declarations.')
243243
console.info(' This way, TypeScript strips them at compile time and your code can run.')
244244
console.info()
245-
console.info(' ', bold('split-stars'), '[--dry] packages... -- sources...')
245+
console.info(' ', bold('split-stars'), '[--dry] packagenames... -- sourcedirs...')
246246
console.info()
247247
console.info(' In Node.js, when a ESM package imports a CommonJS package using "import *"')
248248
console.info(' an extra "default" key may be added around the imported package contents.')

0 commit comments

Comments
 (0)