medikoo / es5-ext

ECMAScript extensions (with respect to upcoming ECMAScript features)
ISC License
168 stars 81 forks source link

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

Open davidwallacejackson opened 2 years ago

davidwallacejackson commented 2 years ago

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 commented 2 years ago

@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 https://github.com/medikoo/es5-ext/issues/30

davidwallacejackson commented 2 years ago

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 commented 2 years ago

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)