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

package.json uses deprecated subpath folder mappings in exports #17

Closed
tbroyer opened this issue Apr 21, 2021 · 8 comments
Closed

package.json uses deprecated subpath folder mappings in exports #17

tbroyer opened this issue Apr 21, 2021 · 8 comments

Comments

@tbroyer
Copy link

tbroyer commented Apr 21, 2021

package.json's exports uses subpath folder mappings (at https://github.com/jaydenseric/extract-files/blob/v9.0.0/package.json#L37), which is deprecated since Node v12.20.0 / v14.13.0, replaced with subpath patterns (which apparently exist since the beginning of exports maps too).
See https://nodejs.org/api/packages.html#packages_subpath_folder_mappings
The main difference is that the required paths have to exactly match, whether you use require() or import to get to the subpath (whereas with subpath folder mappings, the file extension will be automatically added when using require(), but not when using import).

Fwiw, see rollup/plugins#684 (comment) for the details of the Node algorithm applied to the specific case of require("extract-files/public/extractFiles").

@jaydenseric
Copy link
Owner

jaydenseric commented Apr 21, 2021

Yes, I plan to publish updated package exports fields for all my packages soon, perhaps within a day. I spent part of today looking into the best way to do it. It might require a server major change if we move to subpath patterns and the package engines.node field will need to be updated, but I was not sure exactly what Node.js versions support it as that info was missing from the Node.js docs. It turned out to be a bug with the docs, which I submitted a PR to fix: nodejs/node#38324 .

Some things I'll look further into in the morning:

  1. What should the new package engines.node field be.
  2. Is it a good idea to use both subpath patterns and subpath folder mappings at the same time, as a backwards compatible fallback? Does the ordering allow a "fallback" like this?
  3. What exactly should the subpath patterns be? i.e. Whats the difference between "./public/*": "./public/*" and "./public/*": "./public/*.js".
  4. Will subpath patterns affect how deep require and import paths can be used regarding file extensions?

@tbroyer
Copy link
Author

tbroyer commented Apr 21, 2021

Apparently earlier versions of Node will happily ignore subpath patterns: https://github.com/nodejs/node/blob/v12.8.1/doc/api/esm.md#resolution-algorithm, so engine.node could probably stay the same.
And because newer versions will order entries by decreasing length, you could have a subpath pattern falling back to a subpath folder mapping.

As for the difference between "./public/*": "./public/*" and "./public/*": "./public/*.js", with the former only require("extract-files/public/extractFiles.js") (with file extension) will work, whereas with the latter only require("extract-files/public/extractFiles") (without file extension) will work.

Lastly, the * matches anything, including foo/bar.

@jaydenseric
Copy link
Owner

A few things to note…

At first it might seem like a nice idea to configure the package exports to remove the need for users to include the /public/ slug in their require/import paths:

// "./public/": "./public/" (deprecated, as it is now)
import extractFiles from 'extract-files/public/extractFiles.js';
const extractFiles = require('extract-files/public/extractFiles.js');
const extractFiles = require('extract-files/public/extractFiles');

// "./*": "./public/*"
import extractFiles from 'extract-files/extractFiles.js';
const extractFiles = require('extract-files/extractFiles.js');

// "./*": "./public/*.js"
import extractFiles from 'extract-files/extractFiles';
const extractFiles = require('extract-files/extractFiles');

But, then legacy tools that don't understand the package exports rules and only look at the file structure would get confused. For example, it would currently break deep imports from unpkg and jsdelivr:

So, for better compatibility with the wider ecosystem it’s better to stick to:

// "./public/*": "./public/*"
import extractFiles from 'extract-files/public/extractFiles.js';
const extractFiles = require('extract-files/public/extractFiles.js');

// "./public/*": "./public/*.js"
import extractFiles from 'extract-files/public/extractFiles';
const extractFiles = require('extract-files/public/extractFiles');

"./public/*": "./public/*.js" is probably a bad idea, because it only exports files with the .js file extension. What if you want to export a mix of .mjs, .cjs, and .js files?

So, it seems the best option is:

// "./public/*": "./public/*"
import extractFiles from 'extract-files/public/extractFiles.js';
const extractFiles = require('extract-files/public/extractFiles.js');

The sad thing about this is, unlike before, when using require you would have to always specify the file extension in the require path. This would be a server major change.

It would be cool if there is a way to continue supporting require with and without specifying file extensions, but I can't think of a way without hardcoding two rules (with and without the file extension) for every export in the package, which is impractical.

@jaydenseric
Copy link
Owner

Node.js doesn't seem to document array syntax for package export fallbacks, but here is the webpack docs for it:

https://webpack.js.org/guides/package-exports/#alternatives

You would think that Node.js would be able to resolve this, but it can't:

"./public/*": [
  "./public/*",
  "./public/*.mjs",
  "./public/*.cjs",
  "./public/*.js"
]
// Real file path: ./public/foo.js
import foo from 'extract-files/public/foo';
import foo from 'extract-files/public/foo.js';
const foo = require('extract-files/public/foo');
const foo = require('extract-files/public/foo.js');

@tbroyer
Copy link
Author

tbroyer commented Apr 22, 2021

In https://nodejs.org/api/esm.html#esm_resolver_algorithm_specification, PACKAGE_TARGET_RESOLVE, arrays will fallback in case of invalid value or a conditional doesn't match, and in any case without looking up whether the resolved target exists; so an array of strings is useless and will always use the first array value.

jaydenseric added a commit that referenced this issue Apr 23, 2021
@Dolosolow
Copy link

(node:31) [DEP0148] DeprecationWarning: Use of deprecated folder mapping "./public/" in the "exports" field module resolution of the package at /server/node_modules/extract-files/package.json.
Update this package.json to use a subpath pattern like "./public/*".

Not sure what exactly I can do about this. Received this warning in my project.

@jaydenseric
Copy link
Owner

jaydenseric commented May 24, 2021

@Dolosolow it's just a warning, so your project should still function for now. But it's definitely a good idea to update your dependencies so that you're using the latest extract-files, as this was fixed in v10.0.0. Chances are extract-files is a dependency of one of your dependencies, and you didn't install it in your project yourself. If that's the case, you can run npm ls extract-files in your project to see what it's installed under, to help you figure out what dependency needs updating.

@Dolosolow
Copy link

yes it is a dependency of one of my dependencies. Thank you for replying.

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

No branches or pull requests

3 participants