Typescript noEmitOnError in watch mode

[karolis-sh]

• Rollup Plugin Name: @rollup/plugin-typescript
• Rollup Plugin Version: 4.0.0

Feature Use Case

Using rollup in watch mode, the TS compiler shouldn't break the watcher with default options.

Now you need to add typescript({ noEmitOnError: false }) in rollup.config.js for the watcher to work properly.

Feature Proposal

https://github.com/rollup/plugins/tree/master/packages/typescript#noemitonerror the noEmitOnError default should be false, so that rollup usage would be more intuitive.

Ref - #258

[NotWoods]

Agreed, the program should also obey noEmitOnError if specified in the tsconfig.

[shellscape]

@NotWoods I'm doing some housekeeping on old issues. Should we take this one up?

[NotWoods]

I'll take this on.

[benmccann]

I think there are multiple things we can do here:

• The plugin should not override the value of noEmitOnError. This is the action currently suggested
• If noEmitOnError is necessary to make watch mode work, then the plugin should print a warning when that flag is encountered in watch mode or it should be in screaming huge letters on the README along with an explanation as to why. It's not mentioned at all right now
• Investigate if it's possible to make the plugin work in watch mode with either value of noEmitOnError. It's not clear to me why noEmitOnError: true breaks watch mode and if that's necessary and expected

[NotWoods]

noEmitOnError is expected behaviour for most people using the plugin outside of watch mode. They expect Rollup to fail rather than warn when type errors are encountered. We also mention that we default to true in the README. I'll change the default to false when in watch mode.

[benmccann]

I tested noEmitOnError: false with standard building (i.e. without watching) and it still behaves as I'd expect and errors instead of warning. I'm not quite sure why we need to override what the user has in tsconfig.json for the standard case. Perhaps that behavior is different depending on the type of error? I had triggered it by just entering extra characters to create invalid syntax:

import App from './App.svelte';

const app = new App({
	target: document.body,
	props: {
		name: 'world'
	}asdsdf
});

export default app;

I think the behavior you suggested sounds reasonable enough though and will avoid the vast majority of users from hitting this issue

[benmccann]

I see that we also override the defaults for module and skipLibCheck. I did not expect that would be happening and the behavior isn't documented. Would it be okay to remove all the defaults overrides or is there a reason the others are required? (It would obviously be a breaking change requiring a major version bump)

plugins/packages/typescript/src/options/interfaces.ts

Line 9 in 9af578c

export const DEFAULT_COMPILER_OPTIONS: PartialCompilerOptions = {

[benmccann]

PR submitted for this in #544

[shellscape]

Aye I think we'll close this one.