ES module emit uses import specifiers without (required) extensions

[thw0rted]

Category

• Enhancement
• Bug
• Question
• Documentation gap/issue

Version

Please specify what version of the library you are using: [2.0.12]

Please specify what version(s) of SharePoint you are targeting: [N/A]

Expected / Desired Behavior / Question

Consuming code should be able to build as ES modules under latest Webpack

Observed Behavior

Building under Webpack 5.x+ results in errors like

ERROR in ./node_modules/@pnp/sp/content-types/index.js 3:0-16
Module not found: Error: Can't resolve './list' in '....\node_modules\@pnp\sp\content-types'
Did you mean 'list.js'?
BREAKING CHANGE: The request './list' failed to resolve only because it was resolved as fully specified
(probably because the origin is a '*.mjs' file or a '*.js' file where the package.json contains '"type": "module"').
The extension in the request is mandatory for it to be fully specified.
Add the extension to the request.

Steps to Reproduce

Set up a simple project that uses @pnp/sp with a tsconfig that includes

{
  "compilerOptions": {
    "module": "esnext",
    "moduleResolution": "node",
    "target": "es2018",
  }

and compiles using Webpack 5+. I'm using the Angular @ngtools/webpack loader, but I don't think this is caused by the TS loader, I think it's happening after the (consuming) TS is already transpiled.

Details

Webpack 5 has decided to have strong opinions about correctness of import specifiers. Technically, a relative specifier like "./list" is not valid for any native consumer of ES modules -- not in Node, not in any browser. Most consumers probably are passing your library through some kind of further transpiler (likely Babel) that does Node-style module resolution and rewrites everything to CommonJS-style exports, or maybe rewrites the specifiers with an extension. But if we don't pre-process before Webpack tries to make the bundle, it will hit the (unchanged) import { whatever } from "./someFile" and bomb out.

I'm not actually sure how to fix this. There's a very old Typescript issue that still doesn't seem to have a consensus, plus a newer issue that asks to error out instead of emitting extensionless (relative) specifiers. The former issue links to several plugins that will handle automatically generating correct specifiers for you. It also says that you can add a .js extension to the TS import statement, and that will be carried through and "just work", everywhere except TS-Node. I think your advice is already to use the commonJS packages when consuming from Node, so maybe that's the best way forward?

[koltyakov]

Seems that it's a duplicate of this #1461 (what emerged during the discussion in particular). I believe this one was already fixed and on the way to the next release.

[thw0rted]

I just looked through #1461 and I didn't see where it said it was fixed -- the issue was closed because it was originally looking for sample code, and there isn't any, so they gave up. Patrick replied to use the commonjs version because he believed the OP was talking about Node context, but that wasn't the case.

Is there a comment I'm not seeing, where the team said they're going to add extensions to the import statements? (Or some other workaround I'm not aware of?)

[koltyakov]

That's likely the meta info https://twitter.com/mediocrebowler/status/1336673665442848769 (how I knew) maybe not yet committed to the repo(s).

[juliemturner]

This is in progress but it's a very heavy lift as it requires massive updates. We are targeting version 2.1.0, which is in beta now, with the upgrades to support Node 15, Webpack 5, etc... but currently some of the libraries we use, don't support the ext in the imports and so it's not guaranteed to be released with that version.

[thw0rted]

Thanks for the update, Julie. Is the change already being tracked in another issue, or should I just leave this one open?

[PathToSharePoint]

I closed #1461 because my original question was answered and uncovering the bug was a side effect. I was planning on doing more testing before reopening the case.

I didn't know the work was in progress, thanks @koltyakov for sharing the tweet.

@patrick-rodgers @juliemturner how could we help with the heavy lifting? I could do more research on those specific libraries to find out how others are handling the change.

[koltyakov]

Let's keep the issue open then, it does a good job describing the problem explicitly.

[patrick-rodgers]

@thw0rted - To give you a short update on this: making this change is driving me crazy.

To give you slightly longer update:

• I have added ".js" manually to ALL of our imports across test, packages, etc. as it does not look like there will be a tsc flag to help here any time soon. tsc handles this by just loading a .ts file where a .js import is specified. All good here if I do a tsc build.
• Doing the above broke all our tooling: buildsystem, tests, etc.
• ts-node does NOT work with resolving .ts files with .js imports and they are working on it.
• ts-node not working means mocha isn't running right now, which is less than ideal since I have updated all the files and would really like to test them
• our build system is also currently broken due to module issues so I am in the middle of looking at that. Since tsc runs successfully it is just an issue within our stuff now. Not great, but fixable.
• There will be more stuff that doesn't work which we haven't been able to identify yet. Until all the things are resolved we won't release the updated version - just can't do it half way.

What is hard is that node has taken years to come up with this scheme and then broke everyone at once. I understand that folks want to use node 15 and all the things, but the reality is for a large project the change just isn't that simple. It literally affects everything we use and rely on as well as our self-built tooling.

tl;dr; we are working on it, will have it out once it is ready.

[thw0rted]

Patrick, I haven't used them myself but the TS issue about adding extensions links to https://github.com/Zoltu/typescript-transformer-append-js-extension and https://github.com/LeDDGroup/typescript-transform-paths, which are transformers that you can optionally run during tsc execution to modify the emitted code. I think the idea is that instead of hand-jamming extensions on everything, you can configure the transformer to do it for you. This could also solve your Mocha problem, since you could configure the transformer to run when making a packaged build but not run when executing tests. (Technically the code under test would be "different", but only in a well-defined way, right?)

[patrick-rodgers]

No thanks, I'd prefer not to add another hacky thing in the middle of what we are trying to do. My current plan is to fully update the pipeline and have tsc just produce js code, and then we'll run all the tooling against that.

[koltyakov]

Just wondering, how this is solved in some other wildly used projects? Literary question.

I've never ever seen imports with extensions (for js/ts/jsx/tsx) in code base sources out there. Is this a new trend to always follow fully specified paths or an opt-out option for the build tools? Is it only a Webpack 5 demand, do other bundlers going to follow such route in a new iteration?

Sorry for asking all of these, just didn't have a chance to align my knowledge regarding this for a certain level. My personal tend was to switch off Webpack to something 100x times faster. =) But definitely put up with defaults for the most used tools is a thing.

What if to temporary force Webpack 5 with a rule?

{
  test: /\.m?js/,
  resolve: {
    fullySpecified: false
  }
}

[thw0rted]

Honestly I think the answer is that the CJS ecosystem is incredibly deeply entrenched and has been the default way of shipping NPM packages more or less forever. Packages that ship as native ESM are actually pretty rare, and consuming code that stays in native ESM instead of going through a postprocessor (almost certainly Babel) and/or bundler that magically fixes relative imports, is almost unheard of.

FWIW, per the linked Typescript issue, extensions on the specifier are required and "should" have already been used (I never use them either...). We got away with it for so long because we were spoiled by toolchains that allow us to treat our browser code as if it understood node-style module resolution. It never did, and while Chrome has experimental support for the import-maps proposal, that's very much a building-the-plane-mid-flight situation

For today, I added the Webpack rule (fullySpecified) and am leaving it alone, but I think this is a problem that all packages that want to provide native ESM will have to tackle at some point.

[patrick-rodgers]

The answer @koltyakov to what are other packages doing seems to be mixed. Small ones can easily update. Larger ones are trying to figure it out. ts-node is right now the weak link for us. Main issue for me currently is this can either be all es or all cjs. Things don't work if you try and just update pieces - so going through some re-thinking on things.

And the fact that we were technically not supposed to leave off the extensions is weak as there were plenty of folks saying that was the way to go and "better" in the past. Stuff changes, but we should all be honest that this is a breaking change for the entire ecosystem and it will like other similar cases take time to work itself out.

[patrick-rodgers]

Just published a new beta built using all our new tooling and such. Please give it a try for all your node esm needs, everything should now work as expected. All the npm scripts and etc have also been updated and work in node 15 now too. Ended up removing ts-node from the project completely, which is a shame as it is a fantastic thing - but too hard to make it all work. Wrote custom resolvers for mocha and webpack to make things work locally for tests and serving. Anyway, a lot of work but we are better for it now for the long term.

[github-actions]

This issue is locked for inactivity. If you have a related issue please open a new issue and reference this one. Closed issues are not tracked.