Skip to content
This repository has been archived by the owner on Jan 11, 2023. It is now read-only.

Sapper swallows Rollup warnings #1221

Closed
benmccann opened this issue May 22, 2020 · 5 comments · Fixed by #1236
Closed

Sapper swallows Rollup warnings #1221

benmccann opened this issue May 22, 2020 · 5 comments · Fixed by #1236

Comments

@benmccann
Copy link
Member

Describe the bug
Sapper seems to be swallowing Rollup warnings.

It looks to me like Rollup prints warnings by default unless you include --silent: https://rollupjs.org/guide/en/#--silent

To Reproduce
Simply run npm run build on the following project:

https://github.com/benmccann/sapper-swallows-rollup-warnings

Actual behavior

$ npm run build
> Building...
@rollup/plugin-typescript: Couldn't process compiler options

Expected behavior

$ npm run build
> Building...
@rollup/plugin-typescript TS18003: No inputs were found in config file '/home/bmccann/src/sapper-swallows-rollup-warnings/tsconfig.json'. Specified 'include' paths were '["src/**/*"]' and 'exclude' paths were '["node_modules"]'.
> @rollup/plugin-typescript: Couldn't process compiler options

Information about your Sapper Installation:

See https://github.com/benmccann/sapper-swallows-rollup-warnings/blob/master/package.json

$ npx sapper --version
sapper, 0.27.13

Severity
This bug is pure evil. This was so very frustrating and it took me many hours pulling my hair out to deal with Rollup issues as a result. It makes all other Rollup issues 10x worse.

Additional context
I ran into this trying to copy pieces of @babichjacob starter project babichjacob/sapper-firebase-typescript-graphql-tailwindcss-actions-template. I think it's upset at me because I setup TypeScript and haven't added any actual .ts files yet. Now that I've seen the warning I imagine it should be pretty easy to fix the actual issue I'm running into.

@babichjacob
Copy link
Member

So I've spent ~45 minutes looking around to figure out why this is happening.

Here's (probably) the most relevant source to reference: https://github.com/sveltejs/sapper/blob/master/src/core/create_compilers/RollupResult.ts#L29

(That TODO marker is definitely a clue-in).

Right now, Sapper (or the Rollup build process, not really sure how to differentiate them here) is "crashing" (or exiting on purpose) because it encountered the @rollup/plugin-typescript error before the warning that explains it.

A really mediocre (because it introduces console I/O where none was before I think) and quick fix is to add these lines right after the linked line:

for (const warning of compiler.warnings) {
	console.warn(warning.message);
}

This provides the expected output.

Maybe these are each involved project's best responses?:

  1. Someone smarter than me to figure out a better fix than that one there to incorporate into Sapper
  2. @rollup/plugin-typescript to combine their emitted error and warning into one so the failure doesn't get chopped off like this
  3. My template to remove that include field in tsconfig.json (or otherwise accommodate the common situation where somebody doesn't need/have server routes / other .ts files in their project)

(I would also recommend that if you only need TypeScript to start from the leaner template, but in both cases it's a way better idea to use them as bases (i.e. a known working starting point) and replace their contents one-by-one to recreate your pre-existing project)

@benmccann
Copy link
Member Author

Thanks so much for digging into this @babichjacob !

I think the include in your template is just fine. I would have immediately known the issue if the warning had come through. Not to mention I think we should enable writing server.js, client.js, etc. as TypeScript files, which would also remove that issue

Fixing this in Sapper (your option 1) definitely sounds like the best course of action to me.

@benmccann
Copy link
Member Author

It looks like rollup handles different warnings differently. Some it spits out immediately and others it batches and prints at the end:
https://github.com/rollup/rollup/blob/master/cli/run/batchWarnings.ts
https://github.com/rollup/rollup/blob/master/cli/run/watch-cli.ts

There's so much code here I wonder if it actually makes sense to try to copy it all into Sapper or to just execute the actual rollup CLI tool

@benmccann
Copy link
Member Author

Rollup flushes the warnings before it exits when it encounters an error. I've updated #1236 to do the same

@Conduitry
Copy link
Member

Fixed in 0.27.14.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants