Skip to content
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

Support node13 #2451

Merged
merged 6 commits into from Apr 17, 2020
Merged

Support node13 #2451

merged 6 commits into from Apr 17, 2020

Conversation

JoviDeCroock
Copy link
Member

@JoviDeCroock JoviDeCroock commented Apr 8, 2020

In node v13 we either need to end our modular file in .mjs or have a module: true entry in our package json to resolve as a module

@github-actions
Copy link

github-actions bot commented Apr 8, 2020

Size Change: 0 B

Total Size: 39.2 kB

ℹ️ View Unchanged
Filename Size Change
compat/dist/compat.js 3.12 kB 0 B
compat/dist/compat.module.js 3.14 kB 0 B
compat/dist/compat.umd.js 3.17 kB 0 B
debug/dist/debug.js 3 kB 0 B
debug/dist/debug.module.js 2.98 kB 0 B
debug/dist/debug.umd.js 3.08 kB 0 B
devtools/dist/devtools.js 185 B 0 B
devtools/dist/devtools.module.js 194 B 0 B
devtools/dist/devtools.umd.js 260 B 0 B
dist/preact.js 3.83 kB 0 B
dist/preact.min.js 3.83 kB 0 B
dist/preact.module.js 3.84 kB 0 B
dist/preact.umd.js 3.88 kB 0 B
hooks/dist/hooks.js 1.05 kB 0 B
hooks/dist/hooks.module.js 1.08 kB 0 B
hooks/dist/hooks.umd.js 1.12 kB 0 B
test-utils/dist/testUtils.js 437 B 0 B
test-utils/dist/testUtils.module.js 439 B 0 B
test-utils/dist/testUtils.umd.js 515 B 0 B

compressed-size-action

@github-actions
Copy link

github-actions bot commented Apr 8, 2020

Coverage Status

Coverage remained the same at 99.8% when pulling 13fbf84 on node-13 into 7c180cd on master.

package.json Outdated Show resolved Hide resolved
@developit
Copy link
Member

I think I would prefer to try "type":"module" rather than the .mjs extension. Anyone else feel that way?

@JoviDeCroock
Copy link
Member Author

I think I would prefer to try "type":"module" rather than the .mjs extension. Anyone else feel that way?

I wouldn't mind trialing it

@nsivertsen
Copy link

I think I would prefer to try "type":"module" rather than the .mjs extension. Anyone else feel that way?

That route also feels more right to me but iirc that broke CJS consumers when I tried it, because Node will import the CJS entry point but try to parse it as an ES Module.

@@ -28,6 +29,12 @@
"require": "./debug/dist/debug.js",
"import": "./debug/dist/debug.module.js"
},
"./devtools": {
"browser": "./devtools/dist/devtools.module.js",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch about the missing devtools package 👍 💯

@developit
Copy link
Member

@nsivertsen I think we could potentially get around that by renaming the commonjs entry modules to .cjs. Though that may have ill-effects on tools that do file extension / filename-based mapping

@nsivertsen
Copy link

@developit Yeah, I feared that might cause similar problems for tooling as using .mjs, but I don't have any research to back that up. I guess either way this may cause some breakage

Copy link
Member

@marvinhagemeister marvinhagemeister left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome! I'm glad to see those pesky compatibility issues being gone 🙌 Great work all around 💯

@marvinhagemeister marvinhagemeister merged commit a0ef5a1 into master Apr 17, 2020
@marvinhagemeister marvinhagemeister deleted the node-13 branch April 17, 2020 19:31
vikerman added a commit to vikerman/htm that referenced this pull request Apr 18, 2020
Creates conditional export that points "import" conditional exports to mjs files. This lets node load the esm version with having to set "type": "module" on the main package.json.

This is similar to the approach in the main preact repo - preactjs/preact#2451
vikerman added a commit to vikerman/htm that referenced this pull request Apr 26, 2020
Creates conditional export that points "import" conditional exports to mjs files. This lets node load the esm version with having to set "type": "module" on the main package.json.

This is similar to the approach in the main preact repo - preactjs/preact#2451
vikerman added a commit to vikerman/htm that referenced this pull request Apr 30, 2020
Creates conditional export that points "import" conditional exports to mjs files. This lets node load the esm version with having to set "type": "module" on the main package.json.

This is similar to the approach in the main preact repo - preactjs/preact#2451
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants