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
Update package.json #231
base: master
Are you sure you want to change the base?
Update package.json #231
Conversation
Add a extra package.json to the es2015 build that sets the nodejs esm flag if used inside nodejs
Are you able to add a test that ensures this works? I'm not currently qualified to judge this addition. |
@blakeembrey what exactly should i test for?
we create simply a extra package.json inside the dist.es2015 folder with content type: "module" without that package.json this will get treated as commonjs and so it will fail loading. there are other ways like renaming it to .mjs but we do not want to do that as this leads to other incompatiblitys. That is the best fix at the moment you can create that package.json with a other method if you like but i think this one is quick and dirty. |
Less important since I assume this is solved with:
How would someone require this new package that you're adding? Can you add documentation about how it works or how someone might use it to the README? Is it automatically supported by node.js (my understanding was no)? Edit: wouldn't it be preferable to follow https://nodejs.org/api/packages.html#packages_conditional_exports instead? |
@blakeembrey even if you follow that it is needed without that file a import from nodejs is not possible import pathtoregexp from 'path-to-regexp/es2015/*.js' will throw that it gets a esm package but the package is marked as CJS :) the *.js extension with esm content only works when the package.json includes type: module it will recursiv look for the next package.json to verify that. or you rename it to .mjs but that has its own problems :) |
I think during this PR we should also add // package.json
{
...
"type": "commonjs", // explicitly mark all ".js" files in the folder as CJS
"main": "dist/index.js", // used via "require" in legacy node.js
"typings": "dist/index.d.ts",
"module": "dist.es2015/index.js", // used by bundlers (webpack, parcel, rollup, etc.)
"exports": {
"require": "dist/index.js", // used via "require" in modern node.js
"default": "dist.es2015/index.js" // used via "import" in modern node.js
},
...
}
// dist.es2015/package.json
{
"type": "module" // explicitly mark all ".js" files in the folder as ESM
} i.e. users will be able to use both const { pathToRegexp, match, parse, compile } = require("path-to-regexp"); and import { pathToRegexp, match, parse, compile } from "path-to-regexp"; in Node.js depending on their own preferences. |
@frenzzy type fild for cjs is not needed it is already the default if you use a node version that supports it exports should not be a part of this pr this pr simply should allow that you can import the file if you want to import it thats it. i runnend into that and did not want to make a fork for that while i now did a fork of this anyway. i only wanted to backport that. |
Add a extra package.json to the es2015 build that sets the nodejs esm flag if used inside nodejs