Repository navigation
NXT-20954: Added custom typecheck for Typescript 7 #405
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
Changes from all commits
763fde3
bc3baed
408b9ce
63ab97a
4733215
7433117
eb3a265
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,147 @@ | ||
| 'use strict'; | ||
| const {spawn} = require('child_process'); | ||
| const path = require('path'); | ||
| const {WebpackError} = require('webpack'); | ||
|
|
||
| const pluginName = 'EnactTypeScriptCheckPlugin'; | ||
|
|
||
| // Errors reported by this plugin, so `enact serve` can tell them apart from other compilation errors | ||
| const reportedErrors = new WeakSet(); | ||
|
|
||
| // Diagnostics are not reported for test and setup files, matching the fork-ts-checker issue filters. | ||
| const excludeFile = /(^|[\\/])__tests__[\\/]|\.(spec|test)\.[^\\/]+$|(^|[\\/])src[\\/]setup(Proxy|Tests)\.[^\\/]+$/; | ||
|
|
||
| // Parses `tsc --pretty false` output. Each diagnostic starts with `file(line,col): error TSxxxx: message` | ||
| // (or `error TSxxxx: message` for global ones) and any following indented lines elaborate on it. | ||
| function parseDiagnostics (output) { | ||
| const diagnostics = []; | ||
| output.split(/\r?\n/).forEach(line => { | ||
| const located = /^(.+)\((\d+),(\d+)\): (error|warning) (TS\d+): (.*)$/.exec(line); | ||
| const unlocated = !located && /^(error|warning) (TS\d+): (.*)$/.exec(line); | ||
| if (located) { | ||
| const [, file, row, column, category, code, text] = located; | ||
| diagnostics.push({file, row, column, category, code, text}); | ||
| } else if (unlocated) { | ||
| const [, category, code, text] = unlocated; | ||
| diagnostics.push({category, code, text}); | ||
| } else if (line.trim() && diagnostics.length) { | ||
| diagnostics[diagnostics.length - 1].text += '\n' + line; | ||
| } | ||
| }); | ||
| return diagnostics; | ||
| } | ||
|
|
||
| function formatDiagnostic ({file, row, column, code, text}) { | ||
| return (file ? `${file}(${row},${column})\n` : '') + `${code}: ${text}`; | ||
| } | ||
|
|
||
| // Type checks the app with the `tsc` binary for TypeScript versions that do not expose the compiler API | ||
| // required by fork-ts-checker (TypeScript 7+). `tsc` runs alongside each webpack compilation and its | ||
| // diagnostics are added to the compilation once it finishes. | ||
| class TypeScriptCheckPlugin { | ||
| constructor ({tscPath, context, compileOnError = false}) { | ||
| this.tscPath = tscPath; | ||
| this.context = context; | ||
| this.compileOnError = compileOnError; | ||
| this.child = null; | ||
| this.check = null; | ||
| } | ||
|
|
||
| runTsc () { | ||
| this.cancel(); | ||
| const args = [ | ||
| this.tscPath, | ||
| '--project', | ||
| path.join(this.context, 'tsconfig.json'), | ||
| '--noEmit', | ||
| '--skipLibCheck', | ||
| '--incremental', | ||
| '--tsBuildInfoFile', | ||
| path.join(this.context, 'node_modules', '.cache', 'tsconfig.tsbuildinfo'), | ||
| '--pretty', | ||
| 'false' | ||
| ]; | ||
|
|
||
| return new Promise(resolve => { | ||
| let output = ''; | ||
| const child = spawn(process.execPath, args, {cwd: this.context, windowsHide: true}); | ||
| this.child = child; | ||
| child.stdout.on('data', data => (output += data)); | ||
| child.stderr.on('data', data => (output += data)); | ||
| child.on('error', err => { | ||
| resolve({failure: err.message}); | ||
| }); | ||
| child.on('close', (exitCode, signal) => { | ||
| if (this.child === child) this.child = null; | ||
| if (signal) { | ||
| resolve({cancelled: true}); | ||
| } else if (exitCode === 0) { | ||
| resolve({diagnostics: []}); | ||
| } else { | ||
| const diagnostics = parseDiagnostics(output); | ||
| // A non-zero exit without any parsable diagnostics means tsc itself failed | ||
| resolve( | ||
| diagnostics.length ? | ||
| {diagnostics} : | ||
| {failure: output.trim() || `tsc exited with code ${exitCode}`} | ||
| ); | ||
| } | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| cancel () { | ||
| if (this.child) { | ||
| this.child.kill(); | ||
| this.child = null; | ||
| } | ||
| } | ||
|
|
||
| apply (compiler) { | ||
| const start = () => { | ||
| this.check = this.runTsc(); | ||
| }; | ||
| compiler.hooks.run.tap(pluginName, start); | ||
| compiler.hooks.watchRun.tap(pluginName, start); | ||
| compiler.hooks.watchClose.tap(pluginName, () => this.cancel()); | ||
| compiler.hooks.shutdown.tap(pluginName, () => this.cancel()); | ||
|
|
||
| compiler.hooks.afterCompile.tapPromise(pluginName, compilation => { | ||
| // Child compilers inherit this hook; only report on the main compilation | ||
| if (compilation.compiler !== compiler || !this.check) return Promise.resolve(); | ||
|
|
||
| return this.check.then(({cancelled, failure, diagnostics}) => { | ||
| if (cancelled) return; | ||
| if (failure) { | ||
| compilation.warnings.push(new WebpackError(`TypeScript type checking could not run:\n${failure}`)); | ||
| return; | ||
| } | ||
| const errors = []; | ||
| const warnings = []; | ||
| diagnostics | ||
| .filter(({file}) => !file || !excludeFile.test(file)) | ||
| .forEach(diagnostic => { | ||
| const isError = diagnostic.category === 'error' && !this.compileOnError; | ||
| (isError ? errors : warnings).push(formatDiagnostic(diagnostic)); | ||
| }); | ||
|
|
||
| // All diagnostics are reported as a single issue, since react-dev-utils only | ||
| // prints the first error of a compilation in `enact serve` | ||
| const report = (list, issues) => { | ||
| if (!list.length) return; | ||
| const issue = new WebpackError(list.join('\n\n')); | ||
| issue.hideStack = true; | ||
| issues.push(issue); | ||
| return issue; | ||
| }; | ||
| const error = report(errors, compilation.errors); | ||
| if (error) reportedErrors.add(error); | ||
| report(warnings, compilation.warnings); | ||
| }); | ||
| }); | ||
| } | ||
| } | ||
|
|
||
| TypeScriptCheckPlugin.isTypeScriptError = error => reportedErrors.has(error); | ||
|
|
||
| module.exports = TypeScriptCheckPlugin; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,11 +37,12 @@ const { | |
| WebOSMetaPlugin | ||
| } = require('@enact/dev-utils'); | ||
| const createEnvironmentHash = require('./createEnvironmentHash'); | ||
| const TypeScriptCheckPlugin = require('./TypeScriptCheckPlugin'); | ||
|
|
||
| // fork-ts-checker needs the classic TypeScript compiler API. TypeScript 7's | ||
| // package entry only exports a version, so resolution via `main` fails and the | ||
| // checker cannot run. Return null to skip it instead of failing the build. | ||
| function resolveLegacyTypeScript (context) { | ||
| // package entry only exports a version, so for it the `tsc` binary is resolved | ||
| // instead and run by TypeScriptCheckPlugin. Return null when neither is available. | ||
| function resolveTypeScript (context) { | ||
| let typescriptPath; | ||
| try { | ||
| typescriptPath = require.resolve('typescript', {paths: [context]}); | ||
|
|
@@ -51,7 +52,16 @@ function resolveLegacyTypeScript (context) { | |
|
|
||
| try { | ||
| const typescript = require(typescriptPath); | ||
| if (typeof typescript.createProgram === 'function') return typescriptPath; | ||
| if (typeof typescript.createProgram === 'function') return {typescriptPath}; | ||
| } catch (e) { | ||
| // Fall back to the tsc binary below | ||
| } | ||
|
|
||
| try { | ||
| const packagePath = require.resolve('typescript/package.json', {paths: [context]}); | ||
| const {bin} = require(packagePath); | ||
| const tscPath = typeof bin === 'string' ? bin : bin && bin.tsc; | ||
| if (tscPath) return {tscPath: path.join(path.dirname(packagePath), tscPath)}; | ||
| } catch (e) { | ||
| return null; | ||
| } | ||
|
|
@@ -80,10 +90,10 @@ module.exports = function ( | |
|
|
||
| // Check if TypeScript is setup | ||
| const useTypeScript = fs.existsSync('tsconfig.json'); | ||
| const typeScriptPath = useTypeScript ? resolveLegacyTypeScript(app.context) : null; | ||
| if (useTypeScript && !typeScriptPath) { | ||
| const {typescriptPath, tscPath} = (useTypeScript && resolveTypeScript(app.context)) || {}; | ||
| if (useTypeScript && !typescriptPath && !tscPath) { | ||
| console.warn( | ||
| 'Skipping TypeScript type checking during the build. The installed "typescript" package does not provide the compiler API required by fork-ts-checker.' | ||
| 'Skipping TypeScript type checking during the build. Unable to resolve the "typescript" package or its tsc binary.' | ||
| ); | ||
| } | ||
|
|
||
|
|
@@ -652,11 +662,11 @@ module.exports = function ( | |
| // and parses any to copy over any webOS meta assets at build time. | ||
| new WebOSMetaPlugin({htmlPlugin: HtmlWebpackPlugin}), | ||
| // TypeScript type checking | ||
| typeScriptPath && | ||
| typescriptPath && | ||
| new ForkTsCheckerWebpackPlugin({ | ||
| async: !isEnvProduction, | ||
| typescript: { | ||
| typescriptPath: typeScriptPath, | ||
| typescriptPath, | ||
| configOverwrite: { | ||
| compilerOptions: { | ||
| sourceMap: shouldUseSourceMap, | ||
|
|
@@ -692,6 +702,13 @@ module.exports = function ( | |
| infrastructure: 'silent' | ||
| } | ||
| }), | ||
| // TypeScript 7+ type checking via the tsc binary | ||
| tscPath && | ||
| new TypeScriptCheckPlugin({ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TypeScriptCheckPlugin is registered before ESLintPlugin and pushes its diagnostic during afterCompile. In enact serve, createCompiler keeps only errors[0], so an ESLint error from the same compilation is dropped from the terminal and the overlay when a type error is also present. enact pack still prints every error. Worth confirming this is acceptable, since on TypeScript 5/6 fork-ts-checker reports asynchronously and does not take that first slot.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks. The effect is real, but the order is the other way round. eslint-webpack-plugin pushes its error in processAssets (during seal), which runs before afterCompile. So the ESLint error is errors[0], and it was the TypeScript error that got dropped from the terminal. Plugin registration order doesn’t matter here, since the two plugins use different hooks. The browser overlay wasn’t affected: it receives all compilation errors and showed both (checked in the browser). enact pack prints all errors, as you said. It still differed from TS 5/6, where fork-ts-checker printed separately and both errors were visible, so I fixed it. |
||
| tscPath, | ||
| context: app.context, | ||
| compileOnError: process.env.TSC_COMPILE_ON_ERROR === 'true' | ||
| }), | ||
| !noLinting && | ||
| new ESLintPlugin({ | ||
| // Plugin options | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A tsc process stopped on Windows is not treated as cancelled. (optional)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I checked this on Windows (Node 24) with the real child: the TS 7 bin/tsc wrapper that the plugin spawns. After child.kill(), the close event fires with code=null, signal='SIGTERM'. On Windows, libuv records the exit signal when TerminateProcess succeeds, so the result goes through the if (signal) branch and resolves as {cancelled: true}.
I will be beaving this as is