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

cater for change in resolveTypeReferenceDirective API in TypeScript 4.7 #1422

Merged
merged 3 commits into from
Mar 1, 2022

Conversation

johnnyreilly
Copy link
Member

@johnnyreilly johnnyreilly commented Feb 25, 2022

Speculative fix for #1421 - based on @cspotcode's ts-node work here: TypeStrong/ts-node#1648

This has been tested on https://github.com/mjbvz/ts-47-webpack-error mentioned by @mjbvz here: microsoft/TypeScript#48020

Using the initial git clone of the test repo, it errors out as expected:

 john@john-XPS-13-9343  ~/code/github/ts-47-webpack-error   main  yarn
yarn install v1.22.5
info No lockfile found.
warning package-lock.json found. Your project contains lock files generated by tools other than Yarn. It is advised not to mix package managers in order to avoid resolution inconsistencies caused by unsynchronized lock files. To clear this warning, remove package-lock.json.
[1/4] Resolving packages...
[2/4] Fetching packages...
[3/4] Linking dependencies...
[4/4] Building fresh packages...

success Saved lockfile.
Done in 13.70s.
 john@john-XPS-13-9343  ~/code/github/ts-47-webpack-error   main  yarn build
yarn run v1.22.5
$ webpack
assets by status 1.21 KiB [cached] 1 asset
./src/index.ts 39 bytes [built] [code generated] [1 error]

ERROR in ./src/index.ts
Module build failed (from ./node_modules/ts-loader/index.js):
TypeError: path.indexOf is not a function
    at normalizeSlashes (/home/john/code/github/ts-47-webpack-error/node_modules/typescript/lib/typescript.js:7691:26)
    at Object.combinePaths (/home/john/code/github/ts-47-webpack-error/node_modules/typescript/lib/typescript.js:7757:28)
    at /home/john/code/github/ts-47-webpack-error/node_modules/typescript/lib/typescript.js:41935:40
    at Object.firstDefined (/home/john/code/github/ts-47-webpack-error/node_modules/typescript/lib/typescript.js:407:26)
    at primaryLookup (/home/john/code/github/ts-47-webpack-error/node_modules/typescript/lib/typescript.js:41934:27)
    at Object.resolveTypeReferenceDirective (/home/john/code/github/ts-47-webpack-error/node_modules/typescript/lib/typescript.js:41893:24)
    at /home/john/code/github/ts-47-webpack-error/node_modules/ts-loader/dist/servicesHost.js:713:18
    at /home/john/code/github/ts-47-webpack-error/node_modules/ts-loader/dist/servicesHost.js:128:141
    at Array.map (<anonymous>)
    at Object.resolveTypeReferenceDirectives (/home/john/code/github/ts-47-webpack-error/node_modules/ts-loader/dist/servicesHost.js:128:124)

webpack 5.69.1 compiled with 1 error in 750 ms
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

Then when linking in this updated version, it appears to work:

  john@john-XPS-13-9343  ~/code/github/ts-47-webpack-error   main  yarn link "ts-loader"
yarn link v1.22.5
success Using linked package for "ts-loader".
Done in 0.18s.
 john@john-XPS-13-9343  ~/code/github/ts-47-webpack-error   main  yarn build           
yarn run v1.22.5
$ webpack
asset index.js 17 bytes [emitted] [minimized] (name: index)
./src/index.ts 18 bytes [built] [code generated]
webpack 5.69.1 compiled successfully in 3256 ms
Done in 4.40s.

@johnnyreilly
Copy link
Member Author

Have requested feedback from @mjbvz to understand whether this would resolve his issue.

microsoft/TypeScript#48020 (comment)

@johnnyreilly johnnyreilly changed the title speculative fix to #1421 cater for change in resolveTypeReferenceDirective API in TypeScript 4.7 Mar 1, 2022
@johnnyreilly
Copy link
Member Author

We can see that we have failing tests on typescript@next now that typescript@4.6 has shipped.

#1426

With this in place, tests pass. We'll likely ship this as v9.2.7

@johnnyreilly johnnyreilly merged commit f09024d into main Mar 1, 2022
@johnnyreilly johnnyreilly deleted the fix/resolveTypeReferenceDirective branch March 1, 2022 18:27
@mjbvz
Copy link

mjbvz commented Mar 4, 2022

Thanks for the quick fix. This does resolve the issue we were hitting

@dragomirtitian
Copy link

@johnnyreilly Will this version be ported back to 8.x? We are using webpack 4 with ts-loader 8.3 and we are running into this same issue.

@johnnyreilly
Copy link
Member Author

There is a webpack 4 branch https://github.com/TypeStrong/ts-loader/tree/webpack-4

Haven't touched it in a year, but theoretically it's just a case of applying the serviceHost.ts and interfaces.ts changes to it. And the changes are not major.

Publishing old versions is a bit of a faff but doable. If you were able to help out with an initial PR against the webpack 4 branch I could handle the rest?

@johnnyreilly
Copy link
Member Author

Hey @dragomirtitian

I thought I'd try and make things easier by kicking off a new branch cut from the webpack-4 branch and getting it caught up to TypeScript 4.6. It's called: webpack-4-resolveTypeReferences

If you look at this PR:

#1445

You'll see that it's failing for typescript@next (AKA 4.7). The changes from this PR servicesHost.ts and interfaces.ts should be applied to that branch and then (theoretically) the tests for typescript@next will start to pass.

Want to have a crack at it?

@dragomirtitian
Copy link

@johnnyreilly Yeah. Sure. I'll have a look today. Thanks for getting me started :)

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

Successfully merging this pull request may close these issues.

3 participants