DeprecationWarning

[ugrg]

(node:16460) [DEP0148] DeprecationWarning: Use of deprecated folder mapping "./2020/" in the "exports" field module resolution of the package at E:\workspace\trp_v1_app_openmouse\node_modules\es-abstract\package.json. Update this package.json to use a subpath pattern like "./2020/*".

Node version is 16.3.0 webpack version is 4.44.2

[ljharb]

Unfortunately, this is mostly unavoidable. To be able to import es-abstract/2020/RequireObjectCoercible.js in node v12 latest, for example, requires using either a folder mapping or a subpath pattern. However, a subpath pattern would break in node v12.17, which doesn't support subpath patterns.

[ljharb]

The only other alternative would be to add over 1,000 entries to "exports" in package.json to enable deep import of each AO, in both (for example) node v12.17 and v12 latest.

[ljharb]

cc @guybedford this is an unfortunate consequence of the subpath patterns feature combined with the folder deprecation. Is this something that could be fixed somehow?

[jeffrson]

Out of curiosity - why is this a problem? You want/need to support Node12<12.20? And Node14<14.13?

Fixed bugs and security issues should be reasons enough to follow the latest Node-LTS.

In the long term there's probably no other choice anyway...

[davidabap]

my rush build just failed due to this warning, I know rush has the feartue allowWarningsInSuccessfulBuild, but do we have any way to hide this warning ?

[coderaiser]

According to https://gist.github.com/guybedford/4cb418f93ef51f81343d711bbcd80dd5, this helped me to avoid deprecation warning:

-"./helpers": "./helpers/",
+"./helpers/*": "./helpers/*.js",

[ljharb]

@jeffrson i support basically every version of node, yes, and it would be a breaking change to drop support for 12.17.

The vast majority of my packages, including this one, will forever support down to node 0.4, because there's no good reason for them not to.

@coderaiser that syntax would cause node 12.17 to break, and would also cause the existing packages importing those files to break. (example: https://github.com/es-shims/Array.prototype.at/blob/main/index.mjs#L2)

[guybedford]

It's unfortunate that using both syntaxes side-by-side results in the deprecation warning still being hit. We could possibly look into removing the warning in this case, with the pattern taking precedence.

[ljharb]

That would be great.

[guybedford]

@ljharb I actually just checked this again and it does seem like this is what is supported currently - so it should be fine to ship both patterns and path exports together to avoid both the warning and the error on earlier versions.

[guybedford]

The test package.json I was using -

{
  "exports": {
    "./": "./",
    "./*": "./*"
  }
}

with import 'pkg/x.js'.

[ljharb]

@guybedford i tried that, and that doesn’t allow either the extensionless, or the extensioned, path to be imported in both 12.7 and 12.x. It avoids the deprecation warning by breaking one of those buckets.

altho maybe your form works with just extensionless? I’ll check and report back.

[guybedford]

@ljharb right, your outward API contract must be for extensioned paths. There's no way to maintain the unextensioned contract.

[guybedford]

Or rather - you need to pick one or the other for the subpath patterns support.

[ljharb]

@guybedford this breaks requiring of files without extensions as well - so if i used this suggestion, I’d break almost 10% of npm.

If there’s no way to retain extensionless CJS resolution without having the deprecation warning, then that just means everyone on modern node will have to see the deprecation warning.

[ljharb]

I suppose at this point my options are:

1. Keep the deprecation warning for effectively ever
2. Remove the “exports” field entirely, forever
3. Add 1000+ entries to “exports”, also for forever. This will grow by 100-200 every single year, since ES has a yearly edition.

Any chance for an option 4, where node 12/14/16 makes a change so as to support this use case?

[guybedford]

This could be a use case for supporting trailing patterns like:

“exports”: {
  “./*”: “./*.js”,
  “./*.js”: “./*.js”
}

Up until now we haven’t had use cases for the above LHS trailing matches. We would need to carefully define specificity. Ideally it would help to have more than just this use case to motivate with.

On Fri, Jul 30, 2021 at 15:38 Jordan Harband @.***> wrote:

I suppose at this point my options are:

1. Keep the deprecation warning for effectively ever
2. Remove the “exports” field entirely, forever
3. Add 1000+ entries to “exports”, also for forever. This will grow by 100-200 every single year, since ES has a yearly edition.

Any chance for an option 4, where node 12/14/16 makes a change so as to support this use case?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ljharb/es-abstract/issues/133#issuecomment-890222123, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAESFSX6UGDBCAF57V7IKWDT2MSWFANCNFSM5BH5R6AQ .

[ljharb]

I mean, this is the use case I’d expect every single npm package to have whether it wants to support non-latest nodes or not - extensionless deep imports, and matching requires, is the most desirable experience (avoiding the need for treeshaking is hugely important, since treeshaking can never do as good a job as direct deep imports).

[coderaiser]

Remove the “exports” field entirely, forever

@ljharb I think is the best option, to see this deprecation message every time some one uses es-abstract has no sense, and to break the ecosystem is not variant.

How exports helps to current package? As I see all js files should be available, nothing to incapsulate, so better to drop it.

[ljharb]

@coderaiser all tests are shipped with the package; exports means they're not part of the public API. By removing exports, all the tests are part of the API.

[coderaiser]

@coderaiser all tests are shipped with the package

@ljharb why don’t you add them to .npmignore?

[ljharb]

@coderaiser because then npm install foo && npm install && npm test wouldn't work. All of my packages ship all tests, always.

[coderaiser]

@ljharb I don't think that someone will require tests from es-abstract it has no sense for me, also I still don't understand why you publish your packages with tests, if they will be used by no one, but still should be downloaded again and again.

Also we lived somehow without exports feature, and no one required tests, but would be amazing to not see this deprecation message. Of course we can use:

• NODE_NO_WARNINGS=1
• --no-deprecation

But it is a bad practice, so removing exports from package.json (and avoid publishing files that will never run) is much better choise, then ignoring node v16 which will became LTS this October, and node v12 will be deprecated soon.

[ljharb]

I’ve enumerated my options above, they won’t change unless node gives me a fourth option.

[crystalfp]

Thanks! I know it is bad practice, but I modified two lines in your package.json: "./2020/*": "./2020/*.js" and "./helpers/*": "./helpers/*.js" and now I could run eslint clean till next package update.

[corbinu]

@ljharb I have a 4th option and am willing to help out. What if we do a second package. The removal of this issue and the publishing could all be automated with github. Call it lets say "es-abstract-lts" or maybe "@es-abstract/modern" which only supports LTS versions of node going forward and packages that wish to no longer see this warning but don't need support back to node 0.4 can switch

[ljharb]

That would be worse than just doing the breaking change, because it’s twice the maintenance. Also, the primary purpose of this package is polyfills for node versions that aren’t the latest.

[corbinu]

Guess we are going to just have to agree to disagree there. You said you wanted to continue maintaining support for 0.4. It would only be a couple lines of github workflow to maintain and this package as I am sure for most is deep in my dependacy tree so if there was at least an option for some packages to switch that would hopefully fix the issue for those of us just wanting to use tooling that includes this package

[ljharb]

That wouldn’t solve anything though - few people directly depend on this package; it’s a transitive dep, and everything in between also keeps compat with the same node versions.

[corbinu]

It would be solved based on this comment...

@jeffrson i support basically every version of node, yes, and it would be a breaking change to drop support for 12.17.

The vast majority of my packages, including this one, will forever support down to node 0.4, because there's no good reason for them not to.

@coderaiser that syntax would cause node 12.17 to break, and would also cause the existing packages importing those files to break. (example: https://github.com/es-shims/Array.prototype.at/blob/main/index.mjs#L2)

The new package would only support LTS so would support >12.22 not >12.0 so 12.17 would not be an issue. I and others would then file PRs to switch any transitive deps that don't support anything other than currently supported Node releases

[corbinu]

You asked for a 4th option and completely valid one has been given. If you want to add it to the list, reject it and then ask for a 5th that is totally understandable, but to state it is more work/worse than a version bump and then that it wouldn't even work is just objectively false.

[ljharb]

@corbinu i can assure you it's not objectively false. Virtually every single dependent of es-abstract is one of my polyfill packages, all of which must eternally support as far back as possible, including every minor version of node, not just the latest in any major (LTS status has no impact on anything).

I'm going to hide all of these as off topic, since they don't actually solve the issue without a semver-major bump, which is never an option.

[ljharb]

Option 1

Do nothing, but annoy all users of modern node, for the forseeable future. This will likely eventually cause people to make forks of the es-shims ecosystem, or abandon it, which would be a net loss since there doesn't exist a robust and modular a solution as es-shims.

Option 2

Removing the tests as well as "exports" would lose out on encapsulation - likely for ever - and would drop package size from 132.1K to 88.5K, a 33% decrease. However, this would break my npm explore foo && npm install && npm test use case.

Option 3

Object.fromEntries([5].concat(require('./operations/years'), 'helpers').flatMap(y => fs.readdirSync(`./${y}`).flatMap(x => { const ext = `./${y}/${path.basename(x, path.extname(x))}`, noext = `./${y}/${x}`; return [[ext, ext], [noext, ext]]; })))

produces an object containing 1676 entries that I can merge into the "exports" field during the spackle step, which would add 82.3K to package.json - the current package size according to npm pack --dry-run is 132.1K, so this would be a 62% increase. That doesn't seem like an ideal outcome.

[coderaiser]

Let’s vote :)

• do nothing 👎
• remove exports 🚀
• increase package size 😕

[jaydenseric]

The goal of supporting indefinitely every Node.js version (including EOL versions) indefinitely is questionable, but assuming that's a fixed requirement then it's a natural consequence that you can't take advantage of modern Node.js APIs and nice things like encapsulation via the package exports field.

The best thing would be to remove the package exports field, and ideally prevent the publishing of test and other non-production files. This is a best practice anyway for all packages published to npm; it's common sense that if every package published all their tests node_modules would at least double in size. The golden rule of npm package "best practices" is to consider if every package applied a particular practice, would the ecosystem be better or worse off? In my entire career I've never seen someone try to run tests within dependencies installed in their project's node_modules, but bloated node_modules is a thorn in our sides. It's best to whitelist what you do want published by a package files field, but I know from past debates @ljharb strongly favors a blacklist approach via .npmignore. Either way.

As long as the readme clearly articulates what is private and what is public, it's ok to modify or remove parts of the private API in non semver major releases. You could follow the convention of the Deno std lib, which uses _ filename prefixes to denote private parts of the API:

Don't link to / import any module whose path:

Has a name or parent with an underscore prefix: _foo.ts, _util/bar.ts. Is that of a test module or test data: test.ts, foo_test.ts, testdata/bar.txt. Don't import any symbol with an underscore prefix: export function _baz() {}.

These elements are not considered part of the public API, thus no stability is guaranteed for them.

— https://github.com/denoland/deno_std#how-to-use

[ljharb]

@jaydenseric it shouldn’t be, since the entire exports feature was designed with such backwards compatibility in mind by the modules group; the only feature that doesn’t work this way is subpath patterns which of course were built after the modules group dispersed.

I don’t agree the readme makes a difference - whatever is reachable is public, and breaking any of that will break someone, which outweighs any semantic argument about “what the api is documented to be”.

[guybedford]

The benefit of exports encapsulation is CDNs like skypack and JSPM are able to optimize the package for CDN delivery. Especially on a package like this with so many entry points, being able to iterate the public entry points is a crucial need for these service. So I wouldn't discount adding the exports feature so easily since there are big benefits.

It's worth bearing in mind this is a standard brush-up between short-terminism and long-term and there are more than once conversation happening here, as shocking as that might sound (think Stewart Brand layers).

@ljharb I could get behind implementing pattern trailers possibly, it's just about finding the time to do that.

[coderaiser]

@ljharb, I just updated PR #134 according to https://github.com/nodejs/node/issues/38353#issuecomment-825221607, so both node v12 and node v16 is supported and can be published as a patch with a fix, because there is no breaking changes, so:

the wolves are fed and the sheep are safe

[ljharb]

@guybedford That'd be great! Note that this situation is not just a short-term one; it will persist for the duration.

As long as 12, 14, and 16 could either avoid the deprecation warning when both are used, or, add a new feature that would suffice, then that's a much better long-term solution than either of the three above options.

@coderaiser i've commented on the PR to explain how that is still a breaking change.

[ljharb]

v1.18.5 is published with option 2.

I've filed #136 to track re-adding exports and avoiding this warning in the future.

[guybedford]

@ljharb actually thinking about this further I'm not sure trailing patterns would handle all scenarios.

Say we could ship:

{
  "exports": {
    "./": "./",
    "./*": "./*.js",
    "./*.js": "./*.js"
  }
}

then that would support both extensioned and extensionless exports forms, supporting the backwards compatibility you are after.

But, on versions of Node.js that do not supporting trailing patterns, the ./* match would be made not the ./ match so that you would still not be able to support these versions of Node.js, defeating your goal of supporting all versions.

These use cases have definitely made some strong points for supporting trailing patterns and I could still look into that, but unfortunately it won't resolve your issue.

[ljharb]

Yep, unfortunately subpath patterns seem to have boxed me into a permanent corner.

[guybedford]

If we had supported trailing patterns from the start it would have been fine here - agreed that was a missed trick for this use case unfortunately. With regards to moving forward, I'm currently 50/50 with regards to whether I should proceed on such a PR. The cutoff for 12 backports is approaching, further feedback welcome.

[ljharb]

Perhaps the deprecation warning could be removed? It's not actionable for anyone who's seeing it.

[guybedford]

The deprecation was added to ensure consistent syntax, the reason for that hasn't changed here. I'm working on a PR for pattern trailers, hopefully should have something up later today.

[ljharb]

Right, but due to the back compat story it’s not actually ensuring that.

either way, it seems unwise to show warnings to people about “not their own code”.

[quaderi]

Note: node 17+ crashes with DEP0148 instead of warning and node 16 and below is now EOL

Is this package meant only for EOL versions of node at this point? If not, it might make sense to release a new major version of es-abstract that drops backwards compatibility for the very old versions of node that don't support using subpath pattern to be forwards compatible (I could be missing something)

[ljharb]

@quaderi this package works just fine in all versions of node, so i'm not sure what you mean. I ended up removing exports since there's no way for it to sustainably support all required versions of node without issuing a deprecation warning.