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

Move or alias # directories for better ES Modules compatibility #106

Open
davidwallacejackson opened this issue Sep 13, 2021 · 3 comments
Open
Assignees
Labels
question Further information is requested

Comments

@davidwallacejackson
Copy link

I was trying to migrate a create-react-app project to Vite and got import errors related to es5-ext. A bit of investigation led me to this Vite bug. I'm trying to make a PR to fix it, but it's proven unexpectedly challenging and I though it'd be worth opening a ticket on this end as well.

The issue is that Vite, like many other next-generation build tools, uses native ES Modules to load modules in development mode. As a result the browser makes HTTP requests for modules, which leads to requests like GET http://localhost:8080/node_modules/es5-ext/array/#/concat. The hash is interpreted as a URI fragment delimiter, causing the server to receive a request for http://localhost:8080/node_modules/es5-ext/array/ instead.

In theory, this can be solved on Vite's end -- the approach is to replace hash characters in module paths with a URL-safe identifier, then convert back to the hash when the request reaches the server. Doing so, however, makes any code in the request-handling pipeline that does URL-parsing invalid after the replacement is performed. It can be sorted out, but it's a pretty heavy lift. In theory, this is eventually problematic for es5-ext itself -- if a production deployment using ES Modules depends on es5-ext, browsers will have no way to load it.

My understanding is that the hash is used to indicate prototype polyfills. Would there be any problem in moving the prototype polyfills to a URL-safe path like proto, and then leaving # as an alias that imports from the new path for compatibility with existing projects?

@medikoo
Copy link
Owner

medikoo commented Sep 13, 2021

@davidwallacejackson thanks for report. I can imagine how painful it can be.

The way it's being tackled is that new version of this package was published under ext name, and this one is free from this problem.

Still ofc packages that depend on es5-ext, also have to be updated. Unfortunately I do not have ETA on that. Which exactly packages (that depend on `es5-ext) impose an issue on your side?

Note it's technically a duplicate of #30

@medikoo medikoo added the question Further information is requested label Sep 13, 2021
@davidwallacejackson
Copy link
Author

davidwallacejackson commented Sep 13, 2021

Thank you so much for responding so quickly! I'm sorry about missing those existing issues.

I didn't know about ext -- that sounds like the right path forward. There's four or five dependencies in my project using this package, but most of them are dev tooling, so I may not need to change those. The ones that are probably problematic are fantasticon and plotly.js. I'm going to dig in on those and see if their latest versions still use es5-ext -- if they do, I'll open PRs.

Oh, also: would it make sense to put some info about this issue and the new ext package in the project README?

Edit: after investigating fantasticon and plotly.js, it looks like the upstream dependencies that pull in this package are:

If you have time to upgrade them, that'd be awesome! If not, I'll probably submit something myself in the next couple days 🙂

@medikoo
Copy link
Owner

medikoo commented Sep 14, 2021

Thanks @davidwallacejackson

Problem is that also ext have not yet ported some stuff from es5-ext.

As I look in case of es6-weak-map it's only object/set-prototype-of that's missing (other missing things as object/is-value can be taken from type, they are not expected to be provided in ext)

In case of cli-color there's way more, and some imply different alternatives (as a lot of time passed and some native variants emerged in a meantime, e.g. instead of object/map or object/for-each we may refer to object/entries and work with that)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants