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

Add http2 to builtins #1913

Merged
merged 1 commit into from Jul 5, 2019
Merged

Add http2 to builtins #1913

merged 1 commit into from Jul 5, 2019

Conversation

steve-taylor
Copy link
Contributor

@steve-taylor steve-taylor commented Jul 5, 2019

Node.js 10.10.0 added the http2 module. This change adds just enough support for --no-builtins (and therefore --node) to recognise http2 as a Node.js built-in module and not attempt to bundle it.

Copy link
Member

@goto-bus-stop goto-bus-stop left a comment

Choose a reason for hiding this comment

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

Thanks!

@goto-bus-stop goto-bus-stop merged commit 9e3397b into browserify:master Jul 5, 2019
@steve-taylor
Copy link
Contributor Author

steve-taylor commented Jul 5, 2019

You're welcome. Thanks for the quick merge and release!

@ljharb
Copy link
Member

ljharb commented Jul 5, 2019

It might be worth using resolve’s listing of core modules to default all of them to empty (and override with shims as needed) - then this would have Just Worked.

@goto-bus-stop
Copy link
Member

that sounds like a good idea—we've had a few browserify releases lately that were mostly about adding new empty shims.

@ljharb
Copy link
Member

ljharb commented Jul 5, 2019

(Pull in resolve/lib/core, enumerate the entries, and any key whose value is true is a core module in the current node version)

@steve-taylor
Copy link
Contributor Author

The set of core modules isn’t the same for all Node versions. We might not necessarily build and run on the same version of Node.

Maybe we need an extra option to specify the target runtime, e.g. —node-version 10.16.0.

@ljharb
Copy link
Member

ljharb commented Jul 6, 2019

@steve-taylor that’s a reasonable addition to resolve if it’s needed; but i do think that building and running on the same version is typical.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants