vercel / ncc

Compile a Node.js project into a single file. Supports TypeScript, binary addons, dynamic requires.
https://npmjs.com/@vercel/ncc
MIT License
9.13k stars 287 forks source link

build fails with 0.18.5 (worked in 0.17.4) #382

Open tmtron opened 5 years ago

tmtron commented 5 years ago

First, a related question that could help me: Where is the documentation for ncc? The readme file leaves many open question (at least for me as a beginner). And due to this lack of documentation I don't even know how ncc is supposed to work or which questions to ask. So I'm afraid, I must explain the whole use-case.

The build on our CI server was still okay with version 0.17.4 - but now fails with 0.18.5.
To me it seems, that ncc is now somehow finding our top-level tsconfig.json file and it tries to build the whole source code - which will not work, because we have a mono-repo and thus have many different tsconfig.json files.

This is how our code is organized:

The ci script will first do a regular ng build of the server app which creates dist/apps/server/main.js. Then the ci script will cd to apps/server/ and execute ncc build main.js --minify --out ncc to build ncc/index.js. It seems that the newer ncc version will traverse the folder tree up, find the top-level tsconfig.json file and then tries to compile all source code: e.g. in apps/client/... - which will fail, because the top-level tsconfig.json file is not suitable for that source code.

As a workaround I tried to delete the top-level tsconfig.json file - then the build works.

Questions:

  1. Why would ncc even try to start the typescript compiler? it already gets the main.js file?
  2. Why would it use the top-level tsconfig.json, when there is a tsconfig.json file in the current directory?
  3. Can I somehow configure which tsconfig.json file it should use?
styfle commented 5 years ago

It sounds like ncc is trying to read tsconfig.json for both .js and .ts source files when it should only look for tsconfig.json when the source file is .ts (or possibly .tsx.

@guybedford Can you explain why this was changed?

guybedford commented 5 years ago

Nothing in ncc itself that does the tsconfig.json lookup has changed.

But because ncc builds its dependencies of ts-loader and tsconfig-paths, it could be due to an update of these dependencies in the release that cases the problem.

@johnnyreilly would value your thoughts on what this might be that the tsconfig lookup is going further up the folder hierarchy than previously, and if there is a configuration option in ncc that could resolve this?

johnnyreilly commented 5 years ago

Hey @guybedford,

I'm afraid I'm just leaving on my holidays and so I'm not going to spare too much time right now.

I've a couple of quick thoughts before I go though:

Though given the date on this issue maybe that's not the case.

guybedford commented 5 years ago

Thanks for the response here @johnnyreilly, we'll see if we can track it down further here first then.

@tmtron to find the root cause here we'll need to do a bisect on the dependency build. And for that we'll need a reliable replication. If you're able to share a simple repo that demonstrates the problem that would help a lot to get there, otherwise I will come around on this when I'm looking the TypeScript-specific stuff further in about a week or so.

tmtron commented 5 years ago

I don' t have the time to make a reproduction repo now. Bisect is less work, so I've tried that, but came to a strange result.

It seems that I now get the same issue also with ncc 0.17.4 - so my first thought, that the ncc version caused the issue is wrong: it seems that the issue is not caused directly by code changes in ncc, but by some other dependency that has changed.

on our CI-server I found a side-branch where I see one build that still works and the next one fails (on 24th, Aprli, 1 hour apart).

unfortunately the package-lock files are difficult to compare. are there any other dependencies (besides ts-loader) in the package-lock that I should look for? I could also post the package-lock files somewhere if this helps.

tmtron commented 5 years ago

Can someone please confirm my basic understanding of the ncc build:

guybedford commented 5 years ago

for some reason, it also picks up the top-level tsconfig.json file: this is a bug: it should ignore this file, since I feed a main.js file to it and not any ts files

ncc will always read any tsconfig.json file and apply it, even for .js-only builds unfortunately, due to the way we plug in the TypeScript resolver. We could look at only applying the TypeScript resolver when loading a TypeScript module certainly.

tmtron commented 5 years ago

@guybedford thanks for the response.
I can live with deleting my tsconfig files during the ci-build before starting ncc - but it would be great if you could improve the docs, so that this fact is clear - especially for new users.
Feel free to close this issue.