Conversation
src/generate.ts
Outdated
| options.tsconfigRaw.compilerOptions ??= {} | ||
| options.tsconfigRaw.compilerOptions.module = 'preserve' | ||
| options.tsconfigRaw.compilerOptions.moduleResolution = 'bundler' |
There was a problem hiding this comment.
I don't think we should overwrite it, since we'll be reading the tsconfig from the user. In some cases, this could lead to unexpected results, as we've seen in previous issues.
There was a problem hiding this comment.
What are the unexpected results?
FYI I do this in pkgroll and it's been fine: https://github.com/privatenumber/pkgroll/blob/master/src/rollup/configs/dts.ts#L53-L54
Do you prefer we update the tsconfig in the tests instead?
There was a problem hiding this comment.
This is a bit tricky, see https://stackblitz.com/edit/vitejs-vite-wfsxz95t
In general, I’d rather not alter the user’s configuration; if they want this behavior, they can enable it themselves.
There was a problem hiding this comment.
I think this error is technically valid since vue has an export map that disallow importing from vue/jsx-runtime/index.js. But ok, fair enough. Will revert.
src/tsc/index.ts
Outdated
| const preEmitDiagnostics = ts.getPreEmitDiagnostics(program) | ||
| const errors = preEmitDiagnostics.filter( | ||
| (d: ts.Diagnostic) => d.category === ts.DiagnosticCategory.Error, | ||
| ) | ||
| if (errors.length > 0) { | ||
| return { error: ts.formatDiagnostics(errors, formatHost) } | ||
| } |
There was a problem hiding this comment.
Is this API time-consuming? At the very least, we should provide an option for it.
There was a problem hiding this comment.
Previously, yes—good catch. It was always running getPreEmitDiagnostics(program) across the entire program.
Now, it runs program.emit() first, and only runs getPreEmitDiagnostics(program) if emitSkipped indicated that there are further errors to collect.
commit: |
This reverts commit 6613cf1.
Description
Previously, tsc errors were ignored. Since tsc usually emits files even with type errors, it was possible for .d.ts files that don't pass type-checking to be generated.
This PR ensures that type errors are caught as a build error.
This surfaced errors in existing tests (which is great!), so I updated the tsc call to use the following compilerOptions for dts generation:
moduleResolution: "bundler"– most lenient and compatible mode (for bundlers)module: "preserve"– required forbundlermode (es2015+ also works)Linked Issues
Additional context