-
-
Notifications
You must be signed in to change notification settings - Fork 26
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
Bundlers loaders are not utilising diagnostics produced by transformer plugins via ts-patch addDiagnostic utility #139
Comments
@nonara Hey, sorry for bothering, but do you have any thoughts on this one? |
I confirm. I'll add that there's a similar issue with ForkTSChecker |
I conducted some research. For example, I tried using the 'transformProgram': true option. The plugin code looked something like this: export default function (program: Program, host: CompilerHost) {
const { ...newProgram } = program;
return Object.assign(program, {
getSemanticDiagnostics: () => {
// some code
return [];
}
});
} And this works with |
Although such code works with ts-loader @artem1458 fyi |
AFAIK, all ts program factory functions call If you're saying the resulting Program instance isn't your replaced one (and you did a persistent patch), it sounds like the plugin you're using is a different instance of typescript. You could trace out where the plugin code is getting or requiring the typescript module it uses to for program creation. If it's required, you can use Given what you've shared, my biggest suspicion would be that your plugin is using an unpatched installation of typescript. A first fast diagnostic route would be to search node modules for all installations of ts, and if you find one that your plugin is may be using, add a throw to the top of the You might also alter the plugin code where it creates program and look for |
Although... I managed to make it work with fork-ts-checker (by turning off the 'build' option in the fork-ts-checker settings). However, the plugin behaves incorrectly. getSemanticDiagnostics is called only once, and 'sourceFile' comes as undefined. Perhaps, this is an issue with fork-ts-checker. In any case, the approach with addDiagnostic does not work with any loader and plugin (ts-loader, fork-ts-checker, rollup-plugin-typescript2). And, most likely, the method of overriding 'program.getSemanticDiagnostics' is not the most appropriate way. Although, a similar approach is used in the default TypeScript plugins. Additionally, I'll add that ts-loader works even without the 'transformProgram': true option, using code like this: export default function (program: Program, host: CompilerHost, pluginConfig: CliPluginConfig): TransformerPlugin {
program.getSemanticDiagnostics = (sourceFile, cancellationToken) => {
// some code
return [];
}
return () => (sourceFile) => sourceFile;
} However, fork-ts-checker doesn't work at all with this code and behaves incorrectly with the provided one above. |
I figured out with I overrode |
Sorry guys. Not sure why, but GitHub mobile app has been messing up today. I had some messages duplicate and others were deleted entirely.
So what we do pre-transform is hook into I just recalled that some of these applications have their own way of filtering diagnostics. Looks like ts-loader does indeed have that, so it's not surprising that this is the case. (options -> ignoreDiagnostics) Something to bear in mind is that the purpose of ts-patch was to hook into tsc or the compiler API's emit process, which it does accomplish. So, when it comes to certain cases that do their own magic with the TS compiler API, support can't be guaranteed. However... These are popular libraries, and making sure it's supported does make sense. I already have a plan outline for better diagnostics filtering for the upcoming new major version, but maybe we can find an easy route here.
@DiFuks Can you give some more details on what exactly you're doing in your workaround? Also, do you know if |
no :( My workaround looks something like this:{
"transform": "ts-overrides-plugin/cli",
"transformProgram": true,
"overrides": [
{
"files": ["./src/modern/*.{ts,tsx}"],
"compilerOptions": {
"strict": true,
},
},
]
}, import type { PluginConfig } from 'ts-patch';
import { TransformerPlugin } from 'ts-patch/plugin-types';
import { globSync } from 'glob';
import {
CompilerHost,
CompilerOptions,
createProgram,
Program,
} from 'typescript';
import * as path from 'path';
interface Override {
files: string[];
compilerOptions: CompilerOptions;
}
interface CliPluginConfig extends PluginConfig {
overrides: Override[];
}
export default function (program: Program, host: CompilerHost, pluginConfig: CliPluginConfig): TransformerPlugin {
const getSemanticDiagnostics = program.getSemanticDiagnostics.bind(program);
const {overrides} = pluginConfig;
const defaultCompilerOptions = program.getCompilerOptions();
const rootPath = defaultCompilerOptions.project ? path.dirname(defaultCompilerOptions.project) : process.cwd();
const overridesWithProgram = overrides.map((override) => {
const files = globSync(override.files, {
cwd: rootPath,
absolute: true,
});
return {
files,
program: createProgram(files, {
...defaultCompilerOptions,
...override.compilerOptions,
}),
}
});
program.getSemanticDiagnostics = (sourceFile, cancellationToken) => {
const originalDiagnostics = getSemanticDiagnostics(sourceFile, cancellationToken);
// for ForkTsCheckerWebpackPlugin and tspc
if (!sourceFile) {
const overridesDiagnostics = overridesWithProgram.flatMap((override) => {
cancellationToken?.throwIfCancellationRequested()
return override.files.flatMap((file) => {
cancellationToken?.throwIfCancellationRequested()
const sourceFile = override.program.getSourceFile(file);
if (!sourceFile) {
return [];
}
return override.program.getSemanticDiagnostics(sourceFile);
});
});
return [...originalDiagnostics, ...overridesDiagnostics];
}
// for ts-loader
const override = overridesWithProgram.find((override) => {
return override.files.includes(sourceFile.fileName);
});
if (override) {
return override.program.getSemanticDiagnostics(sourceFile);
}
return originalDiagnostics;
}
return () => (sourceFile) => sourceFile;
} Although the original code that worked with tsc looked like this: {
"transform": "ts-overrides-plugin/cli",
"overrides": [
{
"files": ["./src/modern/*.{ts,tsx}"],
"compilerOptions": {
"strict": true,
},
},
]
}, import type { PluginConfig, TransformerExtras } from 'ts-patch';
import { TransformerPlugin } from 'ts-patch/plugin-types';
import { globSync } from 'glob';
import {
CompilerOptions,
createProgram,
Program,
} from 'typescript';
import * as path from 'path';
interface Override {
files: string[];
compilerOptions: CompilerOptions;
}
interface CliPluginConfig extends PluginConfig {
overrides: Override[];
}
export default function (program: Program, pluginConfig: CliPluginConfig, {
diagnostics,
addDiagnostic,
removeDiagnostic,
}: TransformerExtras): TransformerPlugin {
const {overrides} = pluginConfig;
const defaultCompilerOptions = program.getCompilerOptions();
const rootPath = defaultCompilerOptions.project ? path.dirname(defaultCompilerOptions.project) : process.cwd();
const overridesWithProgram = overrides.map((override) => {
const files = globSync(override.files, {
cwd: rootPath,
absolute: true,
});
return {
files,
program: createProgram(files, {
...defaultCompilerOptions,
...override.compilerOptions,
}),
}
});
overridesWithProgram.forEach((override) => {
override.files.forEach((file) => {
const sourceFile = override.program.getSourceFile(file);
if (!sourceFile) {
return;
}
const diagnostics = override.program.getSemanticDiagnostics(sourceFile);
diagnostics.forEach((diagnostic) => {
addDiagnostic(diagnostic);
});
});
});
return () => {
return (sourceFile) => sourceFile;
};
} If you're interested, here's what the native plugin for the IDE looks like: {
"name": "ts-overrides-plugin",
"overrides": [
{
"files": ["./src/modern/*.{ts,tsx}"],
"compilerOptions": {
"strict": true,
},
},
]
}, import ts from 'typescript/lib/tsserverlibrary';
import { globSync } from 'glob';
import path from 'path';
interface Override {
files: string[];
compilerOptions: ts.CompilerOptions;
}
interface IdePluginConfig {
overrides: Override[];
}
export default function init({ typescript }: { typescript: typeof ts}) {
function create(info: ts.server.PluginCreateInfo) {
const { overrides } = info.config as IdePluginConfig;
const defaultCompilerOptions = info.project.getCompilerOptions();
const rootPath = path.dirname(info.project.getProjectName());
const overridesWithProgram = overrides.map((override) => {
const files = globSync(override.files, {
cwd: rootPath,
absolute: true,
});
return {
files,
options: {
...defaultCompilerOptions,
...override.compilerOptions,
},
}
});
return new Proxy(info.languageService, {
get(target, p, receiver): any {
if (p === 'getSemanticDiagnostics') {
return (fileName: string) => {
const override = overridesWithProgram.find((override) => {
return override.files.includes(fileName);
});
if (override) {
const defaultProgram = target.getProgram();
const sourceFile = defaultProgram?.getSourceFile(fileName);
const program = typescript.createProgram([fileName], override.options);
return program.getSemanticDiagnostics(sourceFile);
}
return target.getSemanticDiagnostics(fileName);
}
}
return target[p as keyof ts.LanguageService];
}
})
}
return { create };
} |
Thanks for sharing, @DiFuks! Here are a couple notes that can help: 1. Use the provided You should use This ensures you're using the same typescript as what was loaded initially. 2. Your Program transformer has an incorrect return The "Program Transformer" pattern is meant to take an input It looks like your code returns a source file transformer function pattern. In your case, you'd want to just return the original One general note. The reason I didn't add diagnostics modification functions for Program transformer is because you can do the work yourself by directly dealing with the Program instance. In your use-case, a Program transformer is definitely the correct route, and from what I can tell, your method of doing it looks reasonable. I wouldn't actually advise using a Source Transformer for your purposes. TestBefore you make any changes to your code to address the above, would you mind helping with something? Step 1In your workaround code, keep everything as it is (for now), but make the following changes:
What we did was:
Step 2
Step 3Please report back the outputs of each run |
@nonara Thank you for your help! Here is the report: tspc
ts-loader
fork-ts-checker (build)
Also, I rewrote the plugin based on your advice. Here's the plugin code along with examples. What I found out:
It's evident that the issues are only with I tried to come up with a drastic solution - override the I would really like to bring the implementation of my plugin to completion. Since I believe it would be helpful to many people, I'm willing to assist you in fixing it (if you find it necessary) |
I've made progress in finding a solution. The issue in my case is that getSemanticDiagnostics is called not from the program but from builderProgram. In turn, it calls getSemanticDiagnosticsOfFile. Only then does it call getProgramDiagnostics from the program. getProgramDiagnostics is a private method of the program. I can override it for my needs, but perhaps it will be a somewhat suboptimal workaround |
The final version of the solution For some reason, overriding getProgramDiagnostics also didn't yield results - it returned an empty array. In the end, I had to override getBindAndCheckDiagnostics. As a result, everything worked in ts-loader, tspc, forktschecker, both in build and watch modes. |
@artem1458 Thank you! Oddly, the solution presented here works well with ForkTSChecker. Unfortunately, my team specifically uses ForkTSChecker. :( |
@nonara @DiFuks You know, I actually thinking about adding methods that will add different kind of diagnostics to the program Program have a few methods for providing diagnostics
Compiler plugin developers can decide which diagnostics they want to add. @nonara What do you think about it, is it ok to add such methods? I can add them to fix it if you think that it make sense. |
When using
webpack
withts-loader
andts-patch
- diagnostics produced by transformer plugin is not utilised, the result - compilation is not failing even with reported diagnostics with severity ERROR. Same issue appears when usingvite
withrollup-plugin-typescript2
andts-patch
.This issue appears in both
ts-patch
modes -Live Compiler
andPersistent Patch
.When using
tsc
or ortspc
diagnostics is utilised and failing compilation as expected.Minimal reproduction repo: https://github.com/artem1458/ts-patch-webpack
Output from
build:tsc
script (failed as expected):Output from
build:webpack
script (not failing build):Custom transformer is working Custom transformer is working asset main.js 1.23 KiB [emitted] (name: main) ./src/index.ts 28 bytes [built] [code generated] webpack 5.89.0 compiled successfully in 725 ms
Output from
build:vite
script (not failing build):The text was updated successfully, but these errors were encountered: