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

Please provide es module distributable #174

Closed
jacobg opened this issue Dec 6, 2018 · 8 comments
Closed

Please provide es module distributable #174

jacobg opened this issue Dec 6, 2018 · 8 comments

Comments

@jacobg
Copy link

jacobg commented Dec 6, 2018

Please provide es module distributable in npm, i.e., instead of:

module.exports = pathToRegexp
module.exports.parse = parse
module.exports.compile = compile
module.exports.tokensToFunction = tokensToFunction
module.exports.tokensToRegExp = tokensToRegExp

Do this:

export default pathToRegexp
pathToRegexp.parse = parse
pathToRegexp.compile = compile
pathToRegexp.tokensToFunction = tokensToFunction
pathToRegexp.tokensToRegExp = tokensToRegExp

This is important for webpack applications that have to transpile an ES depedendency that has a transitive dependency on path-to-regexp. More on that issue here:

vuejs/vue-cli#1568

@blakeembrey
Copy link
Member

blakeembrey commented Dec 7, 2018

Nothing in that issue lept out to me as requiring this change. Can you link or specify a specific issue you wish to fix? Webpack definitely works normally with CommonJS and we aren’t going to be moving to ES modules anytime soon since it’s not fully fleshed out under node.js.

@jacobg
Copy link
Author

jacobg commented Dec 7, 2018

Apparently, in Webpack 4, when it transpiles ES code, it must use import for importing ES modules and require for importing CommonJS modules. I'm not clear on the details behind it, but here's a tldr; article: https://medium.com/webpack/webpack-4-import-and-commonjs-d619d626b655

The build script for path-to-regexp could publish both CommonJS and ES modules, and then let the dependent decide which one to use.

@blakeembrey
Copy link
Member

@jacobg What build script would that be? There's isn't one today.

Also, according to the linked article this should also be fine. It's just use-case A1. If you can build a reproduction I will take a look.

@jacobg
Copy link
Author

jacobg commented Dec 7, 2018

@blakeembrey I mean adding a build script. Here is an example simple project where the source is common js, but the build also adds an es module:
https://github.com/dobromir-hristov/get-value

I honestly don't understand the issue exactly, but a lot of people are waving their hands that it is a webpack issue. For me it's tldr; Here is a link to a webpack issue on it:

webpack/webpack#4039

@blikblum
Copy link

Hi, just come with a need for this feature not related to webpack at all.

I'm trying to ditch the need to do a build (with rollup) just to test (through karma) a library i develop. Since all code written with ES modules i could just load it, with minimal changes, with current browsers but there's a dependency on path-to-regexp which does not have an es dist

I'm willing to do the work but needs some guidance since it will require some technical decisions.

The more common way would be write the main source code with ES module and run a build step with babel to convert to commonjs

Another way would write a custom build tool (with a few regex) to convert current code to an ES module version.

I can do any of the above

BTW: both can coexist easily with module/main package entries

@blikblum
Copy link

Since was really easy i just did a PR: #190

@daKmoR
Copy link

daKmoR commented Aug 30, 2019

es modules become more and more important... 🤗

what would it take to publish it to also support es modules? how about going the same way sinon did?

e.g. in your package.json

"main": "index.js",
"module": "index-es.js",

and then the only difference between those files is the export syntax
=> could of course even be automated but not sure if it's worth it

or what do you think is needed for you to support es modules as well?

@blakeembrey
Copy link
Member

This has been worked on in #203.

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

Successfully merging a pull request may close this issue.

4 participants