Use caches for module resolution and type reference directives when using compiler default functions #1287

sheetalkamat · 2021-04-20T01:22:48Z

Uses #microsoft/TypeScript#43700

…dule resolution till watch is invoked

JasonKleban · 2021-04-21T20:42:33Z

@johnnyreilly could this please be also get applied to version 8.x for webpack 4 compatibility?

JasonKleban · 2021-04-21T20:43:15Z

What is the technique for testing both this and the typescript branch locally at once?

johnnyreilly · 2021-04-21T20:47:48Z

@johnnyreilly could this please be also get applied to version 8.x for webpack 4 compatibility?

If you wanted to do a PR to backport it against the https://github.com/TypeStrong/ts-loader/tree/webpack-4 branch after this has been merged then we could potentially look at it.

sheetalkamat · 2021-04-21T20:48:54Z

@johnnyreilly I am not sure what the test failures mean but i am able to successfully run the tests locally.
This is ready to merge pending review.

sheetalkamat · 2021-04-21T20:50:47Z

@JasonKleban I had to manually create links for typescript built locally while I was working on this and ts-loader as well and use it in your sample repro. That's how I tested it, not sure if that's how you would need to test but bot should post typescript build from the PR at microsoft/TypeScript#43700 (comment)

johnnyreilly · 2021-04-21T20:54:14Z

@johnnyreilly I am not sure what the test failures mean but i am able to successfully run the tests locally.
This is ready to merge pending review.

Thanks @sheetalkamat - looking at the test failure it looked like a random yarn failure. I've requeued and will try to make time to do the review soon. Thanks for your efforts!

JasonKleban · 2021-04-21T21:24:31Z

@sheetalkamat can you share any indication for how big of a performance impact you expect this to have? Might it nearly equalize ts-loader & webpack to tsc compilation times?

sheetalkamat · 2021-04-21T21:48:00Z

Might it nearly equalize ts-loader & webpack to tsc compilation times?

@JasonKleban probably.. The sample was such a small, that I couldn't measure it reliably especially with my local setup but i did verify that lot of file accesses and checks get reduced (esp you can see that in trace logs). Because of the way ts-loader works(i am not sure if it is intentional or not but I didn't go about investigating that) i have seen that program is created with some subset of files and more files get added to it as webpack asks about more information about dependencies.. So in that second pass, you start reusing module resolution, type reference resolution, package.json cache etc... Note that package.json improvement can be seen (from trace logs) for semver types as well as it tries to read package.json many times the first time itself.

Btw typescript PR build is available at microsoft/TypeScript#43700 (comment)

src/interfaces.ts

src/servicesHost.ts

johnnyreilly

This looks great! Fine work @sheetalkamat!

I'm happy for this to be released and I've updated the package.json / CHANGELOG.md accordingly.

sokra · 2021-04-22T06:32:14Z

test/comparison-tests/projectReferencesSymLinks/expectedOutput-4.1/patch0/output.txt

@@ -6,10 +6,10 @@ asset ../../lib/tsconfig.tsbuildinfo 2.73 KiB [emitted]
 cached modules 408 bytes [cached] 2 modules
 ./src/index.ts 108 bytes [built] [code generated] [1 error]

-ERROR in app/src/index.ts
+ERROR in app\src\index.ts


hmm? why is this a backslash?

btw. webpack usually would display this as ERROR in ./app/src/index.ts

I suspect this is just ts-loader surfacing up what TypeScript hands over:

ts-loader/src/utils.ts

Line 77 in c2d1ca2

const file = diagnostic.file;

So if you're running on a Windows machine it's likely \ country

…ves when using compiler default functions" TypeStrong#1287 to v8

* Backport "Use caches for module resolution and type reference directives when using compiler default functions" #1287 to v8 * regen comparison tests * Revert "regen comparison tests" This reverts commit f04c5cf. * only the two failing tests rerun. * yarn run comparison-tests -- --save-output --single-test projectReferencesSymLinks * yarn run comparison-tests -- --single-test projectReferencesSymLinks_WatchApi --save-output Co-authored-by: Sheetal Nandi <shkamat@microsoft.com> Co-authored-by: John Reilly <johnny_reilly@hotmail.com>

felixmosh · 2021-05-02T07:13:41Z

I'm getting an error with v9.1.0-9.1.1, since this is the only change in these versions, I post it here

[webpack-cli] HookWebpackError: Cannot read property 'length' of undefined
    at makeWebpackError (/Users/***/Projects/node/***/node_modules/webpack/lib/HookWebpackError.js:49:9)
    at /Users/***/Projects/node/***/node_modules/webpack/lib/Compilation.js:2496:12
    at eval (eval at create (/Users/***/Projects/node/***/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:26:1)
    at fn (/Users/***/Projects/node/***/node_modules/webpack/lib/Compilation.js:427:17)
    at Hook.eval [as callAsync] (eval at create (/Users/***/Projects/node/***/node_modules/tapable/lib/HookCodeFactory.js:33:10), <anonymous>:24:1)
    at Hook.CALL_ASYNC_DELEGATE [as _callAsync] (/Users/***/Projects/node/***/node_modules/tapable/lib/Hook.js:18:14)
    at cont (/Users/***/Projects/node/***/node_modules/webpack/lib/Compilation.js:2493:34)
    at /Users/***/Projects/node/***/node_modules/webpack/lib/Compilation.js:2539:10
    at /Users/***/Projects/node/***/node_modules/neo-async/async.js:2830:7
    at Object.each (/Users/***/Projects/node/***/node_modules/neo-async/async.js:2850:39)
-- inner error --
TypeError: Cannot read property 'length' of undefined
    at computeLineStarts (/Users/***/Projects/node/***/node_modules/typescript/lib/typescript.js:9580:27)
    at Object.getLineStarts (/Users/***/Projects/node/***/node_modules/typescript/lib/typescript.js:9640:60)
    at getCurrentLineMap (/Users/***/Projects/node/***/node_modules/typescript/lib/typescript.js:101516:59)
    at emitDetachedCommentsAndUpdateCommentsInfo (/Users/***/Projects/node/***/node_modules/typescript/lib/typescript.js:105227:94)
    at emitBodyWithDetachedComments (/Users/***/Projects/node/***/node_modules/typescript/lib/typescript.js:105067:17)
    at emitSourceFile (/Users/***/Projects/node/***/node_modules/typescript/lib/typescript.js:103774:21)
    at pipelineEmitWithHint (/Users/***/Projects/node/***/node_modules/typescript/lib/typescript.js:101597:24)
    at noEmitNotification (/Users/***/Projects/node/***/node_modules/typescript/lib/typescript.js:100109:9)
    at onEmitNode (/Users/***/Projects/node/***/node_modules/typescript/lib/typescript.js:85991:13)
    at onEmitNode (/Users/***/Projects/node/***/node_modules/typescript/lib/typescript.js:96421:17)
caused by plugins in Compilation.hooks.processAssets
TypeError: Cannot read property 'length' of undefined
    at computeLineStarts (/Users/***/Projects/node/***/node_modules/typescript/lib/typescript.js:9580:27)
    at Object.getLineStarts (/Users/***/Projects/node/***/node_modules/typescript/lib/typescript.js:9640:60)
    at getCurrentLineMap (/Users/***/Projects/node/***/node_modules/typescript/lib/typescript.js:101516:59)
    at emitDetachedCommentsAndUpdateCommentsInfo (/Users/***/Projects/node/***/node_modules/typescript/lib/typescript.js:105227:94)
    at emitBodyWithDetachedComments (/Users/***/Projects/node/***/node_modules/typescript/lib/typescript.js:105067:17)
    at emitSourceFile (/Users/***/Projects/node/***/node_modules/typescript/lib/typescript.js:103774:21)
    at pipelineEmitWithHint (/Users/***/Projects/node/***/node_modules/typescript/lib/typescript.js:101597:24)
    at noEmitNotification (/Users/***/Projects/node/***/node_modules/typescript/lib/typescript.js:100109:9)
    at onEmitNode (/Users/***/Projects/node/***/node_modules/typescript/lib/typescript.js:85991:13)
    at onEmitNode (/Users/***/Projects/node/***/node_modules/typescript/lib/typescript.js:96421:17)
error Command failed with exit code 2.

sheetalkamat added 4 commits April 15, 2021 10:37

ClearCache for watchHost

5b1e7ec

Use module resolution cache

76c7a93

Use complete ResolvedModuleFull from typescript

f55dd9f

Use type reference directive cache so that results are atomic like mo…

28a903d

…dule resolution till watch is invoked

This was referenced Apr 20, 2021

Repeated package.json reads in ts-loader ts api wrapper compared to tsc --build microsoft/TypeScript#42670

Closed

Module Resolution and Type Reference directive cache updates and its API changes microsoft/TypeScript#43700

Merged

sheetalkamat added 2 commits April 20, 2021 12:00

Merge branch 'main' into cache

a39786a

Some updates

749c00b

sheetalkamat marked this pull request as ready for review April 20, 2021 19:06

Test baseline update

8df84b2

johnnyreilly reviewed Apr 22, 2021

View reviewed changes

src/interfaces.ts Outdated Show resolved Hide resolved

johnnyreilly reviewed Apr 22, 2021

View reviewed changes

src/servicesHost.ts Outdated Show resolved Hide resolved

Update src/servicesHost.ts

317a72a

johnnyreilly reviewed Apr 22, 2021

View reviewed changes

src/servicesHost.ts Outdated Show resolved Hide resolved

johnnyreilly added 4 commits April 22, 2021 05:11

Update src/servicesHost.ts

b29866c

Update CHANGELOG.md

dc339b3

Update package.json

1e49142

Update src/interfaces.ts

2573331

johnnyreilly approved these changes Apr 22, 2021

View reviewed changes

johnnyreilly mentioned this pull request Apr 22, 2021

Use caches for module resolution and type reference directives when using compiler default functions TypeStrong/fork-ts-checker-webpack-plugin#598

Closed

johnnyreilly merged commit 0d735e5 into main Apr 22, 2021

johnnyreilly deleted the cache branch April 22, 2021 05:38

sokra reviewed Apr 22, 2021

View reviewed changes

JasonKleban pushed a commit to JasonKleban/ts-loader that referenced this pull request Apr 22, 2021

Backport "Use caches for module resolution and type reference directi…

5435096

…ves when using compiler default functions" TypeStrong#1287 to v8

JasonKleban mentioned this pull request Apr 22, 2021

Backport #1287 to v8 #1291

Merged

johnnyreilly mentioned this pull request Jun 10, 2021

Project references and v8 (webpack v4): JS files in node_modules should not be type checked #1338

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use caches for module resolution and type reference directives when using compiler default functions #1287

Use caches for module resolution and type reference directives when using compiler default functions #1287

sheetalkamat commented Apr 20, 2021

JasonKleban commented Apr 21, 2021

JasonKleban commented Apr 21, 2021

johnnyreilly commented Apr 21, 2021

sheetalkamat commented Apr 21, 2021

sheetalkamat commented Apr 21, 2021

johnnyreilly commented Apr 21, 2021

JasonKleban commented Apr 21, 2021

sheetalkamat commented Apr 21, 2021 •

edited

Loading

johnnyreilly left a comment

sokra Apr 22, 2021

johnnyreilly Apr 22, 2021

felixmosh commented May 2, 2021

Use caches for module resolution and type reference directives when using compiler default functions #1287

Use caches for module resolution and type reference directives when using compiler default functions #1287

Conversation

sheetalkamat commented Apr 20, 2021

JasonKleban commented Apr 21, 2021

JasonKleban commented Apr 21, 2021

johnnyreilly commented Apr 21, 2021

sheetalkamat commented Apr 21, 2021

sheetalkamat commented Apr 21, 2021

johnnyreilly commented Apr 21, 2021

JasonKleban commented Apr 21, 2021

sheetalkamat commented Apr 21, 2021 • edited Loading

johnnyreilly left a comment

Choose a reason for hiding this comment

sokra Apr 22, 2021

Choose a reason for hiding this comment

johnnyreilly Apr 22, 2021

Choose a reason for hiding this comment

felixmosh commented May 2, 2021

sheetalkamat commented Apr 21, 2021 •

edited

Loading