Skip to content
This repository has been archived by the owner on May 2, 2022. It is now read-only.

Improve module compatibility #88

Merged
merged 2 commits into from
Oct 4, 2020
Merged

Improve module compatibility #88

merged 2 commits into from
Oct 4, 2020

Conversation

mraerino
Copy link
Member

@mraerino mraerino commented Oct 4, 2020

Which problem is this pull request solving?

Edge Handlers will provide browser apis if they can. We should choose the browser entrypoint of any packages if it is provided.

Some modules are built for node, but can run fine on our runtime, if you provide shims.

Describe the solution you've chosen

Enable browser: true in the resolve module.

Set preferBuiltins: false so people can install their own shims if they want.

Add the node-globals and node-builtins rollup plugins.

Describe alternatives you've considered

We could require people to install shims themselves.

Checklist

  • The status checks are successful (continuous integration). Those can be seen below.

@mraerino mraerino added the type: feature code contributing to the implementation of a feature and/or user facing functionality label Oct 4, 2020
Copy link
Contributor

@calavera calavera left a comment

Choose a reason for hiding this comment

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

sounds like a good idea

@mraerino mraerino merged commit 7414aa7 into master Oct 4, 2020
@mraerino mraerino deleted the fix/better-module-compat branch October 4, 2020 17:48
@ehmicky
Copy link
Contributor

ehmicky commented Oct 5, 2020

Great!

@mraerino
Copy link
Member Author

mraerino commented Oct 5, 2020

this actually has some bugs, because those plugins are unmaintained and use an outdated version of acorn, so you can't use es2020 syntax, which edge handlers would support ootb

@mraerino
Copy link
Member Author

mraerino commented Oct 5, 2020

@mraerino mraerino mentioned this pull request Oct 5, 2020
@ehmicky
Copy link
Contributor

ehmicky commented Oct 5, 2020

@mraerino Should we create an issue around this if this is a known problem?

@mraerino mraerino mentioned this pull request Oct 5, 2020
1 task
@mraerino
Copy link
Member Author

mraerino commented Oct 5, 2020

fixed in #93

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
type: feature code contributing to the implementation of a feature and/or user facing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants