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

Module not found for TypeScript 4.7 #1463

Closed
unional opened this issue May 31, 2022 · 54 comments
Closed

Module not found for TypeScript 4.7 #1463

unional opened this issue May 31, 2022 · 54 comments

Comments

@unional
Copy link
Contributor

unional commented May 31, 2022

Expected Behaviour

Able to bundle.

Actual Behaviour

errors such as:

Module not found: Error: Can't resolve './tersible.js'

Steps to Reproduce the Problem

In TypeScript 4.7, the NodeNext module resolution requires the source code to specify the extension as .js.
e.g.:

// index.ts
export * from './tersible.js' // for `tersible.ts`

For ts-jest on the jest side,
it solves that by doing a transformation:

  moduleNameMapper: {
    '^(\\.{1,2}/.*)\\.js$': '$1',
  },

Maybe something similar is needed?
Would be great to add a section in the readme specifically for that.

Location of a Minimal Repository that Demonstrates the Issue.

Not a minimal repo, but here is an example: https://github.com/unional/tersify.
Run yarn bundle in the repo

🌷

@johnnyreilly
Copy link
Member

It's possible that ts-loader doesn't support ECMAScript modules for importing with extensions as you are attempting. Certainly no work has been done to actively support this.

We're open to this support being added, though it's not clear at the moment where work would be required. Some questions:

If someone would like to start work on this, we'll happily take a look and collaborate on PRs ❤️🌻

@unional
Copy link
Contributor Author

unional commented May 31, 2022

I have found a workaround for the time being:

  plugins: [
    new NormalModuleReplacementPlugin(/.js$/, (resource) => {
      resource.request = resource.request.replace(/.js$/, '')
    })
  ],

🍻

but I am hitting two other problems:

  • exports not defined (from unpartial cjs/index.js)
  • the good old source map for library issue (in the past I make it work with inlineSourcemap in the library's tsconfig.json, but you know that's not a good solution for production).

I've created this branch to test the issues: https://github.com/unional/tersify/tree/webpack
This can be tested by opening the demo.html after running yarn bundle

Link to unpartial: https://github.com/unional/unpartial (still remember that lib? 😄 )

@unional
Copy link
Contributor Author

unional commented May 31, 2022

are changes required to ts-loader? If so, what's the nature of the required change?

Although the workaround is done at the webpack config's plugins, I think it should be handled by ts-loader, as it would be very cumbersome to instruct user to use ts-loader AND do that workaround.

@unional
Copy link
Contributor Author

unional commented May 31, 2022

Just in case I have tested older code of that repo. It does look like the exports not defined issue is related to the new stuff.

@cspotcode
Copy link
Contributor

Roughly the three major bits of NodeNext / mts / cts support in ts-node were:


Replace node's resolver with one that checks what I call "alternative file extensions." Every time node's resolver would check for foo.js, also check for the alternatives: foo.ts, foo.tsx, foo.jsx


Teach transpiler codepaths to emit Typescript's 2x new flavors of module:

  • nodenext-flavored CommonJs, like commonjs emit except that now dynamic imports are preserved
  • nodenext-flavored ESM, like EsNext emit except that now import foo = require('foo') statements automatically use createRequire to gin up a require function

ts.transpileModule does not expose this functionality; cannot be used


Teach transpiler codepaths to classify files as either CJS or ESM using the same rules as NodeNext

  • So, use file extension and contextual package.json to decide which of the 2x flavors to emit
  • TypeScript relies on its own resolver to do this classification; transpile-only codepaths need to reimplement it

@johnnyreilly
Copy link
Member

Thanks @cspotcode - that's super helpful! @unional it seems like you're super motivated; do you want to have a crack at this? Don't worry too much about tests etc for now; if you can get something working, that's first prize. We can handle the rest later.

@unional
Copy link
Contributor Author

unional commented Jun 1, 2022

Yeah, I can give it a shot. But need to handle a few things first. Will see if I can get to it. :)

@unional
Copy link
Contributor Author

unional commented Jun 1, 2022

FYI here is one issue that I found related to source map:
microsoft/TypeScript#49335

@unional
Copy link
Contributor Author

unional commented Jun 1, 2022

new NormalModuleReplacementPlugin(/.js$/, (resource) => {
      if (/node_modules/.test(resource.context)) return
      resource.request = resource.request.replace(/.js$/, '')
    })

I found that the workaround needs to be adjusted to this so that it will not remove the .js in external deps.
Of course, the node_modules is really a workaround and doesn't work in other cases (e.g. yarn 2)

Also, it doesn't work when transpileOnly is used:

        options: {
          transpileOnly: true
        }

My guess is because it can't follow the references.

UPDATE: This only works when using the Nodenext module resolution. If using node, it doesn't work.
Getting exports not defined error at runtime.

@unional
Copy link
Contributor Author

unional commented Jun 2, 2022

@johnnyreilly I notice running yarn test causes the yarn.lock updated.
Is there a particular version of yarn this repo is using? I have 1.22.15.

Maybe first have a PR to pin the yarn version by using packageManager (corepack)?

UPDATE:
and one test actually failed: 3.6.0_projectReferencesToBeBuilt,
and test/execution-tests/pnpm/pnpm-lock.yaml also updated.

The error detail is:

Error during file loading or preprocessing
TypeError: Cannot read properties of undefined (reading 'split')

Trying again in the docker

UPDATE2:

 > [4/4] RUN apt-get update && apt-get -y install google-chrome-stable     && rm -rf /var/lib/apt/lists/*:                           
#7 0.377 Get:1 http://deb.debian.org/debian bullseye InRelease [116 kB]                                                              
#7 0.377 Get:2 http://security.debian.org/debian-security bullseye-security InRelease [44.1 kB]                                      
#7 0.425 Get:3 https://dl.yarnpkg.com/debian stable InRelease [17.1 kB]                                                              
#7 0.446 Get:4 http://deb.debian.org/debian bullseye-updates InRelease [39.4 kB]                                                     
#7 0.485 Get:5 http://dl.google.com/linux/chrome/deb stable InRelease [1811 B]
#7 0.527 Get:6 http://security.debian.org/debian-security bullseye-security/main arm64 Packages [152 kB]
#7 0.602 Get:7 http://deb.debian.org/debian bullseye/main arm64 Packages [8070 kB]
#7 0.649 Get:8 https://dl.yarnpkg.com/debian stable/main all Packages [10.9 kB]
#7 0.841 Get:9 https://dl.yarnpkg.com/debian stable/main arm64 Packages [10.9 kB]
#7 1.144 Get:10 http://deb.debian.org/debian bullseye/non-free arm64 Packages [69.6 kB]
#7 1.155 Get:11 http://deb.debian.org/debian bullseye/contrib arm64 Packages [40.8 kB]
#7 1.161 Get:12 http://deb.debian.org/debian bullseye-updates/main arm64 Packages [2600 B]
#7 1.805 Fetched 8575 kB in 2s (5592 kB/s)
#7 1.805 Reading package lists...
#7 2.123 Reading package lists...
#7 2.425 Building dependency tree...
#7 2.513 Reading state information...
#7 2.571 E: Unable to locate package google-chrome-stable

um... 🤔

@unional
Copy link
Contributor Author

unional commented Jun 2, 2022

microsoft/TypeScript#49340

Related issue. Maybe when that is available, ts-loader can use it to resolve the module name instead of coming up with a custom implementation.

unional added a commit to unional/tersify that referenced this issue Jun 2, 2022
TypeStrong/ts-loader#1463

`transpileOnly: true` doesn't work
`tsconfig.cjs.json` doesn't work
Use `NormalModuleReplacementPlugin` as workaround
unional added a commit to unional/tersify that referenced this issue Jun 2, 2022
TypeStrong/ts-loader#1463

`transpileOnly: true` doesn't work
`tsconfig.cjs.json` doesn't work
Use `NormalModuleReplacementPlugin` as workaround
unional added a commit to unional/tersify that referenced this issue Jun 2, 2022
TypeStrong/ts-loader#1463

`transpileOnly: true` doesn't work
`tsconfig.cjs.json` doesn't work
Use `NormalModuleReplacementPlugin` as workaround
unional added a commit to unional/tersify that referenced this issue Jun 2, 2022
TypeStrong/ts-loader#1463

`transpileOnly: true` doesn't work
`tsconfig.cjs.json` doesn't work
Use `NormalModuleReplacementPlugin` as workaround
@johnnyreilly
Copy link
Member

johnnyreilly commented Jun 2, 2022

The pnpm test is very out of date - I wouldn't worry particularly about it; it needs refreshing anyway. It's surfaced misleading results in the past as well. It's on my list to do something about.

The error message is familiar - I think it came up recently but I can't remember the details.

loading or preprocessing
TypeError: Cannot read properties of undefined (reading 'split')

As to the yarn version; we use v1 but not a specific version. Not a problem if yarn.lock is updated.

Apologies if I only reply occasionally - I'm on holiday

@unional
Copy link
Contributor Author

unional commented Jun 2, 2022

Apologies if I only reply occasionally - I'm on holiday

NP. That explains all those nice pics. 🌷

@unional
Copy link
Contributor Author

unional commented Jun 2, 2022

microsoft/TypeScript#49083

Cross ref

@johnnyreilly
Copy link
Member

I'm back home now @unional - is everything going okay on this? Are you blocked on anything?

@unional
Copy link
Contributor Author

unional commented Jun 10, 2022

Hi there. I was trying to do a clean run of tests but there is a test failure. Wanted to have a clean start. At the moment I was making some changes to one of my projects as my office work needs it. Was planning to get on this after that.

As for the approach, I saw 4.7.3 have the resolve function exposed for ts-node. My guess is if ts-loader can use the same function, that would the the way to go.

@johnnyreilly
Copy link
Member

Sounds good - when you get to it, feel free to stick up a work in progress PR with broken tests if needs be. Will be happy to pair on this

@unional
Copy link
Contributor Author

unional commented Jun 11, 2022

Have a question about the test system. Is there any reason why it is so slow? Is it because of using karma? (Haven't use karma for ages. 😛 ).

If there are things can be done to improve on that (e.g. migrate to jest or use some other tools), I might able to help too. :)

This is obvious off-topic to this OP. So if you are interested, we can kick off a discussion about it. 🌷

@unional
Copy link
Contributor Author

unional commented Jun 11, 2022

When running the tests in Windows environment, I got this error:

START:
Webpack bundling...
Error: EINVAL: invalid argument, mkdir 'C:\Users\homa\AppData\Local\Temp\_karma_webpack_583487\D:\code\others\ts-loader\test\execution-tests\3.6.0_projectReferencesToBeBuilt\lib'

Seems like the test setup doesn't work in Windows.

@johnnyreilly
Copy link
Member

So there's two test systems; both quite old. The execution tests which run TypeScript tests using Karma and validate if transplantation is working successfully. And comparison tests which are effectively equivalent to Jest snapshot tests. @g-plane tried to do this but we couldn't get it to work on windows - see discussion here:

#1161

@unional
Copy link
Contributor Author

unional commented Jun 11, 2022

I see. I can take a look at it. :)

@johnnyreilly
Copy link
Member

Seems like the test setup doesn't work in Windows.

They should work on Windows: https://github.com/TypeStrong/ts-loader/runs/6682852223?check_suite_focus=true

See the above GitHub Action from 10 days ago running on Windows

@johnnyreilly
Copy link
Member

There's a chore piece of work to do with the comparison tests every time there's a major release of TypeScript. I don't think I've done it yet for 4.7. will probably do it on Sunday night

@unional
Copy link
Contributor Author

unional commented Jun 11, 2022

$ git clean -xfd test/comparison-tests && npm link ./test/comparison-tests/testLib && node test/comparison-tests/run-tests.js
npm ERR! code ERESOLVE
npm ERR! ERESOLVE could not resolve
npm ERR!
npm ERR! While resolving: [email protected]
npm ERR! Found: [email protected]
npm ERR! node_modules/webpack
npm ERR!   peer webpack@"^5.0.0" from [email protected]
npm ERR!   node_modules/karma-webpack
npm ERR!     dev karma-webpack@"^5.0.0" from the root project
npm ERR!   peer webpack@"^5.1.0" from [email protected]
npm ERR!   node_modules/terser-webpack-plugin
npm ERR!     terser-webpack-plugin@"^5.1.1" from [email protected]
npm ERR!   3 more (webpack-cli, @webpack-cli/configtest, the root project)
npm ERR!
npm ERR! Could not resolve dependency:
npm ERR! peer webpack@"2 || 3" from [email protected]
npm ERR! node_modules/babel-loader
npm ERR!   dev babel-loader@"^7.0.0" from the root project
npm ERR!
npm ERR! Conflicting peer dependency: [email protected]
npm ERR! node_modules/webpack
npm ERR!   peer webpack@"2 || 3" from [email protected]
npm ERR!   node_modules/babel-loader
npm ERR!     dev babel-loader@"^7.0.0" from the root project
npm ERR!
npm ERR! Fix the upstream dependency conflict, or retry
npm ERR! this command with --force, or --legacy-peer-deps
npm ERR! to accept an incorrect (and potentially broken) dependency resolution.
npm ERR!
npm ERR! See /home/homa/.npm/eresolve-report.txt for a full report.

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/homa/.npm/_logs/2022-06-11T06_10_04_151Z-debug-0.log
error Command failed with exit code 1.

When trying to run on WSL. 🤣 looks fun.

@unional
Copy link
Contributor Author

unional commented Jun 11, 2022

See the above GitHub Action from 10 days ago running on Windows

I see. Likely because my WIndows env is using Node 18. Let me downgrade

@johnnyreilly
Copy link
Member

Gosh what would cause that 😂 I use Ubuntu for the most part - which should behave the same but I've not encountered that

@johnnyreilly
Copy link
Member

Inside the devcontainer it looks like we use node 16:

FROM mcr.microsoft.com/vscode/devcontainers/javascript-node:16

@johnnyreilly
Copy link
Member

And node 14 and 16 in our execution test pack:

node: [14, 16]

@unional
Copy link
Contributor Author

unional commented Jun 11, 2022

Running the tests with Node 16 now. btw, running test with Node 16 in WSL still fail the save way as above. Just FYI. 🌷

@johnnyreilly
Copy link
Member

Strange. Just out of curiosity, why don't you fork ts-loader, submit a PR that just changes whitespace or something and see if the tests run successfully there? It'll validate if tests are fundamentally broken. I don't think they are but you never know

@johnnyreilly
Copy link
Member

Running the tests with Node 16 now. btw

Oh they run - cool

@johnnyreilly
Copy link
Member

I wonder if this was the issue: https://stackoverflow.com/a/64615583/761388

@unional
Copy link
Contributor Author

unional commented Jun 11, 2022

Oh they run - cool

Ar, I mean "running". 🤣 . It failed. Yeah, trying to figure out what's wrong.

Right now I'm suspecting nushell, but not very likely. :)

@unional
Copy link
Contributor Author

unional commented Jun 11, 2022

Strange. Just out of curiosity, why don't you fork ts-loader, submit a PR that just changes whitespace or something and see if the tests run successfully there? It'll validate if tests are fundamentally broken. I don't think they are but you never know

Will do.

@unional
Copy link
Contributor Author

unional commented Jun 11, 2022

image

🤣 that's why. Only running comparison tests on Windows. I was running yarn test, i.e. also running execution-tests

@johnnyreilly
Copy link
Member

#1464

It looks like the npm that ships with node 18 had some breaking changes. We use yarn 1 for ts-loader anyway, so I'd advise sticking with that

@unional
Copy link
Contributor Author

unional commented Jun 11, 2022

Do you use Prettier + ESLint:
image
image

or Prettier ESLint:
image

My personal coding style is not compatible with the coding style in the repo.

I can add a vscode setting to the repo so that it is easier for contributors to create consistent code following the style of the repo.

Let me know which one do you use or do you have any other extensions preferences.

@johnnyreilly
Copy link
Member

Prettier + ESLint I think

My personal coding style is not compatible with the coding style in the repo.

Not sure I understand? Come what may, I think we have a precommit hook set up to format consistently. Generally we use the prettier defaults.

@johnnyreilly
Copy link
Member

I can see you've upgraded to TS 4.7.3 - the comparison tests won't work now until I do some fiddling. Execution tests will be fine though

@unional
Copy link
Contributor Author

unional commented Jun 11, 2022

Prettier + ESLint I think

👌

The issue is when using VSCode and doing format document will mess up the styling. I do that through muscle memory so always need to [ctrl+z] afterwards. So it's much better if the environment is setup already so that it doesn't mess up the code.

the comparison tests won't work now until I do some fiddling.

Ok. The all the tests "passes" in Windows.
Please go ahead to make any changes needed. I updated it so that I can add the NodeNext test cases for this issue.

@johnnyreilly
Copy link
Member

The issue is when using VSCode and doing format document will mess up the styling. I do that through muscle memory so always need to [ctrl+z] afterwards. So it's much better if the environment is setup already so that it doesn't mess up the code.

I'm open to adjusting this - I mostly care that code is formatted consistently. If there's DX tweaks you have in mind we can look at it.

Please go ahead to make any changes needed. I updated it so that I can add the NodeNext test cases for this issue.

I'll try and do this tomorrow night. Will pop a message on here when done

@unional
Copy link
Contributor Author

unional commented Jun 11, 2022

I'm open to adjusting this - I mostly care that code is formatted consistently. If there's DX tweaks you have in mind we can look at it.

#1477 is the PR for that.

@johnnyreilly
Copy link
Member

upgrade to 4.7.3 (including test pack) complete

@unional
Copy link
Contributor Author

unional commented Jun 13, 2022

I have published color-map with a few variations to test this:

@unional
Copy link
Contributor Author

unional commented Jun 15, 2022

justland/just-web#58

One update. After I add the cjs/package.json workaround, at least the CommonJS side resolution is working fine.
It's not directly related to this issue,
but it shows that having due-export (CJS + ESM/NodeNext) able to work at the end application.

@unional
Copy link
Contributor Author

unional commented Jun 16, 2022

This is an additional plugin that seems to solve the same problem:

https://github.com/softwareventures/resolve-typescript-plugin

@johnnyreilly
Copy link
Member

Yeah I'm aware of this. I have an idea that we used to link to this somewhere

@Hujianboo
Copy link

Hujianboo commented Jul 17, 2022

hi, does this problem have been solved by ts-loader or do I have to use other tool to handle it?

@unional
Copy link
Contributor Author

unional commented Jul 17, 2022

Right now you can workaround it using the workaround mentioned above or using the plugin mentioned above.

I don't know what should the general direction for this at the moment, as I just learn that ts-loader is not using tsc for resolution.

What do you think @johnnyreilly ?

@johnnyreilly
Copy link
Member

I'm not sure to be honest. @djcsdy's plugin implies webpack itself didn't support ESM. My guess is that might have changed by now but I don't know. If it has, maybe there's less of a workaround way forward?

I'd appreciate anyone who's up to date with where webpack is with this advising? cc @sokra / @alexander-akait

@alexander-akait
Copy link
Contributor

hm, why don't use https://webpack.js.org/configuration/resolve/#resolveextensionalias? I think it will work

@slhck
Copy link

slhck commented Jul 22, 2022

This is an additional plugin that seems to solve the same problem:

https://github.com/softwareventures/resolve-typescript-plugin

This in conjunction with ts-loader actually did the job for me just fine.

Sorry for the +1 without contributing much. Really just coming in here as a user who wanted to convert their project to ESM and is still quite confused about how the different standards work and tools interact.

@IronGeek
Copy link

hm, why don't use https://webpack.js.org/configuration/resolve/#resolveextensionalias? I think it will work

Yes, it works!
Upgrading webpack to v5.74.0 and adding the resolve.extensionAlias settings, fix this issue for me 🎉

@unional
Copy link
Contributor Author

unional commented Aug 1, 2022

microsoft/TypeScript#49083 (comment)

🎉

@unional unional closed this as completed Aug 1, 2022
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

7 participants