path-to-regexp icon indicating copy to clipboard operation
path-to-regexp copied to clipboard

Update package.json

Open frank-dspeed opened this issue 5 years ago • 7 comments

Add a extra package.json to the es2015 build that sets the nodejs esm flag if used inside nodejs

frank-dspeed avatar Sep 16 '20 17:09 frank-dspeed

Coverage Status

Coverage remained the same at 100.0% when pulling b5fdc8540dd81bc129457663c31501fc76f51b54 on frank-dspeed:patch-1 into d8bc41fd5f81d3d9bde0ad0266b8b7a7ea4066e7 on pillarjs:master.

coveralls avatar Sep 16 '20 17:09 coveralls

Are you able to add a test that ensures this works? I'm not currently qualified to judge this addition.

blakeembrey avatar Oct 03 '20 04:10 blakeembrey

@blakeembrey what exactly should i test for?

  • That the file gets created with right content?
  • That nodejs and other is tools are able to load the esm version?

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.

frank-dspeed avatar Oct 03 '20 07:10 frank-dspeed

That the file gets created with right content?

Less important since I assume this is solved with:

That nodejs and other is tools are able to load the esm version?

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 avatar Oct 11 '20 23:10 blakeembrey

@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 :)

frank-dspeed avatar Oct 12 '20 08:10 frank-dspeed

I think during this PR we should also add type and exports fields to the main package.json so all the possible cases will work:

// 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 avatar Oct 12 '20 09:10 frenzzy

@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.

frank-dspeed avatar Oct 12 '20 15:10 frank-dspeed