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

Vague failed to transpile error -- noEmitOnError: true breaks rpt2 #254

Closed
benmccann opened this issue Nov 20, 2020 · 7 comments · Fixed by #338, #345 or #406
Closed

Vague failed to transpile error -- noEmitOnError: true breaks rpt2 #254

benmccann opened this issue Nov 20, 2020 · 7 comments · Fixed by #338, #345 or #406
Assignees
Labels
kind: bug Something isn't working properly problem: removed issue template OP removed the issue template without good cause problem: stale Issue has not been responded to in some time topic: TS Compiler API Docs Related to the severely lacking TS Compiler API Docs topic: type-only / emit-less imports Related to importing type-only files that will not be emitted

Comments

@benmccann
Copy link

benmccann commented Nov 20, 2020

What happens and why it is wrong

I get the error:

[!] (plugin rpt2) Error: failed to transpile 'packages/kit/src/runtime/navigation/index.ts'

This is almost impossible to debug. Luckily I figured out I can do npx tsc and get the real error message:

src/runtime/navigation/types.ts:33:9 - error TS2304: Cannot find name 'Route'.

33  route: Route;
           ~~~~~


Found 1 error.

Environment

Versions
npx: installed 1 in 1.212s

  npmPackages:
    rollup: ^2.32.0 => 2.32.0 
    rollup-plugin-typescript2: ^0.29.0 => 0.29.0 
    typescript: ^4.0.3 => 4.0.3

Steps to reproduce

If you do not have pnpm then first run npm install -g pnpm

git clone git@github.com:sveltejs/kit.git
cd kit
git reset --hard 2483ae3730a7219814cc654c03b0409a797de2ee
pnpm install
pnpm -r build

After this, you can build just the relevant sub-project by running cd packages/kit and then npx rollup -c for faster debugging

Other context

Looks like it's coming from here:

this.error(red(`failed to transpile '${id}'`));

printDiagnostics right above looks to be being passed an empty array of diagnostics

@agilgur5
Copy link
Collaborator

Per the downstream issue, it seems like OP believed this might be caused by #234 , with pnpm somehow swallowing the diagnostic errors

@agilgur5 agilgur5 added problem: stale Issue has not been responded to in some time topic: monorepo / symlinks Related to monorepos and/or symlinks (Lerna, Yarn, PNPM, Rush, etc) labels May 2, 2022
@agilgur5 agilgur5 added the problem: removed issue template OP removed the issue template without good cause label May 26, 2022
@agilgur5 agilgur5 changed the title Vague "failed to transpile" error Vague "failed to transpile" error when using pnpm May 26, 2022
@agilgur5 agilgur5 changed the title Vague "failed to transpile" error when using pnpm Vague failed to transpile error when using pnpm May 26, 2022
@agilgur5
Copy link
Collaborator

agilgur5 commented May 27, 2022

I fixed pnpm (and symlinks in general) in #332, so I re-visited this to see if it solves the issue here, since pnpm was mentioned downstream.

It was a good thing that the commit hash was included in the issue so that I could go back in time to the right spot.

I tried upgrading rpt2 to latest and added the fix in #332 locally to node_modules, but neither of those fixed it, errors were still not reported. I removed some of the rpt2 config to simplify things and added clean: true (in case of a cache issue), but those changes didn't fix it either. I also set verbosity: 3 (which the issue template requests the log from) and indeed no diagnostics are printed before this error. The diagnostics array is also indeed empty 🤔

So, as far as I can tell, I don't think this is related to #234 , as the fix for it does not fix this issue, unfortunately.

@agilgur5 agilgur5 changed the title Vague failed to transpile error when using pnpm Vague failed to transpile error during fatal error when using pnpm May 29, 2022
@agilgur5 agilgur5 removed the topic: monorepo / symlinks Related to monorepos and/or symlinks (Lerna, Yarn, PNPM, Rush, etc) label May 29, 2022
@agilgur5 agilgur5 changed the title Vague failed to transpile error during fatal error when using pnpm Vague failed to transpile error message after fatal error May 29, 2022
@agilgur5
Copy link
Collaborator

agilgur5 commented May 29, 2022

Investigated a little bit more and both semantic and syntactic diagnostics return empty. These are provided by the TS LanguageService, so not something that rpt2 has any role in.

Checked one step out, the snapshot, and I found:

snapshot:  StringScriptSnapshot {
  text: "export { default as goto } from './goto';\n" +
    "export { default as prefetch } from './prefetch';\n" +
    "export { default as prefetchRoutes } from './prefetchRoutes';\n" +
    "export { default as start } from './start';"
}

from id kit/packages/kit/src/runtime/navigation/index.ts.

That is indeed what that file looks like and there are indeed no errors in that file... so the lack of diagnostic output is correct in that case.

The error from tsc that OP mentioned comes from kit/packages/kit/src/runtime/navigation/types.ts, which is a transitive import of index.ts (some of the files it imports then import types.ts).

So what this suggests the problem is is that on a fatal error, only the first file reported on will show diagnostics, and not its transitive imports that may actually be the cause of the error.

Annnd this seems to be another case where the lacking TS Compiler docs have no mention or example of this case what-so-ever. So unsurprising that rpt2 doesn't handle it given that it's not even mentioned in the docs. rpt2 handles it the same way as the docs do 😕

EDIT: See below, this is actually due to noEmitOnError: true (which well, also isn't mentioned in TS Compiler docs, but none of the overrides a plugin like rpt2 needs are for that matter)

@agilgur5
Copy link
Collaborator

agilgur5 commented May 29, 2022

One more try and I seem to have found root cause finally!
The problem is with the noEmitOnError: true setting in your tsconfig. This defaults to false.

When this is removed, rpt2 passes, and your adjust-typings plugin errors instead.

rpt2 has the setting abortOnError for this (which may duplicate #62), which actually defaults to true because skipping a TS error may cause even more confusing Rollup errors down-the-line (since it's a pipeline, not just a single parser like tsc).

There's also an old, stale feature request, #168 , that requests all errors be reported before aborting, instead of just the first error. That seems related/like what this might duplicate. Although I actually think your use-case is already handled and doesn't require that, as tsc only reports one type-error anyway.

That being said, since rpt2 defaults abortOnError to true, you would think removing noEmitOnError: true wouldn't make a difference, but it actually does in 2 ways:

  1. noEmitOnError is an emit setting, and so it acts like noEmit when there's an error: i.e. all files will have emitSkipped set to true, which is how rpt2 currently determines if an error is fatal or not.
    So we'll want to force noEmitOnError to false actually. We effectively do this anyway since abortOnError is true by default and doesn't relate to the noEmitOnError setting, it's just that the noEmit portion was probably not expected to break things like this and is very infrequently set in tsconfigs, so hasn't popped up till now.

  2. types.ts is a type-only / emit-less TS file, so rpt2 passes and actually doesn't error out at all when noEmitOnError is removed. That is due to Declarations not generated for type-only files not explicitly specified in tsconfig #211 and other long-standing type-only issues

So once 1. is fixed, this will duplicate #211

@agilgur5 agilgur5 added kind: bug Something isn't working properly topic: type-only / emit-less imports Related to importing type-only files that will not be emitted labels May 29, 2022
@agilgur5 agilgur5 changed the title Vague failed to transpile error message after fatal error Vague failed to transpile error, noEmitOnError: true breaks rpt2 May 29, 2022
@agilgur5 agilgur5 self-assigned this May 29, 2022
@agilgur5 agilgur5 changed the title Vague failed to transpile error, noEmitOnError: true breaks rpt2 Vague failed to transpile error -- noEmitOnError: true breaks rpt2 May 29, 2022
@agilgur5
Copy link
Collaborator

agilgur5 commented Jun 2, 2022

Forcing noEmitOnError: false has been released in 0.32.0!

For type-only imports, refer to #211 for progress

@agilgur5
Copy link
Collaborator

agilgur5 commented Jun 5, 2022

Actually, the type-only portion of this should be fixed by #345 as this use-case was using tsconfig include globs (duplicating #298). There's more fixes to come for different situations with type-only files though.

@agilgur5 agilgur5 added the topic: TS Compiler API Docs Related to the severely lacking TS Compiler API Docs label Jul 5, 2022
@agilgur5
Copy link
Collaborator

#345 has been released in 0.33.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind: bug Something isn't working properly problem: removed issue template OP removed the issue template without good cause problem: stale Issue has not been responded to in some time topic: TS Compiler API Docs Related to the severely lacking TS Compiler API Docs topic: type-only / emit-less imports Related to importing type-only files that will not be emitted
Projects
None yet
2 participants