Skip to content

Commit 184652c

Browse files
committed
Use noEmitOnError=false internally.
See microsoft/TypeScript#24444 for the reason. Errors are detected by checking the diagnostic types and emit is skipped "manually", depending on that check result.
1 parent 5bf35aa commit 184652c

File tree

3 files changed

+62
-44
lines changed

3 files changed

+62
-44
lines changed

src/compiler.ts

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,19 @@ export function compile(script: string, compilerOptions?: ts.CompilerOptions, am
2323

2424
// set default compiler options
2525
compilerOptions = compilerOptions || {};
26+
compilerOptions.moduleResolution = ts.ModuleResolutionKind.NodeJs;
27+
// Don't emit faulty code (by default)
2628
if (compilerOptions.noEmitOnError == null) compilerOptions.noEmitOnError = true;
2729
// emit declarations if possible
2830
if (compilerOptions.declaration == null) compilerOptions.declaration = true;
29-
compilerOptions.moduleResolution = ts.ModuleResolutionKind.NodeJs;
31+
32+
// According to https://github.com/Microsoft/TypeScript/issues/24444#issuecomment-392970120
33+
// combining noEmitOnError=true and declaration=true massively increases the work done
34+
// by the compiler. To work around it, we call the compiler with noEmitOnError=false
35+
// and use the actual value to determine if we continue with the emit
36+
const internalOptions = Object.assign({}, compilerOptions, {
37+
noEmitOnError: false,
38+
} as ts.CompilerOptions);
3039

3140
// provide the source file in the virtual fs
3241
const fs = new VirtualFileSystem();
@@ -38,20 +47,20 @@ export function compile(script: string, compilerOptions?: ts.CompilerOptions, am
3847
}
3948

4049
// create the virtual host
41-
const host = new InMemoryHost(fs, compilerOptions);
50+
const host = new InMemoryHost(fs, internalOptions);
4251
// create the compiler and provide nodejs typings
4352
const allFiles = [
4453
"@types/node/index.d.ts",
4554
...Object.keys(ambientDeclarations),
4655
SCRIPT_FILENAME,
4756
];
48-
const program = ts.createProgram(allFiles, compilerOptions, host);
57+
const program = ts.createProgram(allFiles, internalOptions, host);
4958

5059
// compile the script
5160
const emitResult = program.emit();
5261

5362
// diagnose the compilation result
54-
const rawDiagnostics = compilerOptions.noEmitOnError ? emitResult.diagnostics : ts.getPreEmitDiagnostics(program);
63+
const rawDiagnostics = internalOptions.noEmitOnError ? emitResult.diagnostics : ts.getPreEmitDiagnostics(program);
5564
const diagnostics = rawDiagnostics.map(diagnostic => {
5665
let lineNr = 0;
5766
let charNr = 0;
@@ -76,12 +85,14 @@ ${type.toUpperCase()}: ${description}`;
7685
});
7786

7887
const hasError = (
79-
(!diagnostics.every(d => d.type !== "error") || emitResult.emitSkipped)
88+
(
89+
diagnostics.find(d => d.type === "error") != null
90+
|| (emitResult.emitSkipped && !compilerOptions.emitDeclarationOnly)
91+
)
8092
&& compilerOptions.noEmitOnError
8193
);
8294
let result: string;
8395
const resultFilename = SCRIPT_FILENAME.replace(/ts$/, "js");
84-
8596
let declarations: string;
8697
const declarationsFilename = SCRIPT_FILENAME.replace(/ts$/, "d.ts");
8798
if (!hasError && fs.fileExists(resultFilename)) result = fs.readFile(resultFilename);

src/index.test.ts

Lines changed: 31 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ const options = {
1414
};
1515

1616
describe("compiler => ", function() {
17-
1817
this.timeout(30000);
1918

2019
it("it should not explode", () => {
@@ -66,40 +65,40 @@ declare global {
6665
it("it should force ambient declarations to be .d.ts files", () => {
6766
expect(() => compile("", null, { "global.ts": "" })).to.throw();
6867
});
68+
});
6969

70-
describe("performance check =>", () => {
71-
it("compile()", function() {
72-
this.timeout(10000);
73-
const ambient = fs.readFileSync("./test/ioBroker.d.ts", "utf8");
74-
let result: CompileResult;
75-
for (let i = 0; i < 5; i++) {
76-
result = compile(
77-
`const buf = Buffer.alloc(${i} + 1);
78-
console.log(buf.length)`,
79-
null, { "global.d.ts": ambient },
80-
);
81-
expect(result.success).to.be.true;
82-
// about 700ms per call
83-
}
84-
});
70+
describe("performance check =>", function() {
71+
this.timeout(30000);
8572

86-
it("service host", () => {
87-
const tsserver = new Server(options);
88-
const ambient = fs.readFileSync("./test/ioBroker.d.ts", "utf8");
89-
tsserver.provideAmbientDeclarations({ "global.d.ts": ambient });
90-
let result: CompileResult;
91-
for (let i = 0; i < 5; i++) {
92-
log("starting compilation", "info");
93-
result = tsserver.compile("index.ts",
94-
`const buf = Buffer.alloc(${i} + 1);
73+
it("compiler", () => {
74+
const ambient = fs.readFileSync("./test/ioBroker.d.ts", "utf8");
75+
let result: CompileResult;
76+
for (let i = 0; i < 5; i++) {
77+
result = compile(
78+
`const buf = Buffer.alloc(${i} + 1);
9579
console.log(buf.length)`,
96-
);
97-
log("compilation done!", "info");
98-
expect(result.success).to.be.true;
99-
expect(result.declarations).to.equal("declare const buf: Buffer;\r\n");
100-
// about 4ms per call (after the 1st one)
101-
}
102-
});
80+
null, { "global.d.ts": ambient },
81+
);
82+
expect(result.success).to.be.true;
83+
// call durations: ~1200..900 ms
84+
}
10385
});
10486

87+
it("service host", () => {
88+
const tsserver = new Server(options);
89+
const ambient = fs.readFileSync("./test/ioBroker.d.ts", "utf8");
90+
tsserver.provideAmbientDeclarations({ "global.d.ts": ambient });
91+
let result: CompileResult;
92+
for (let i = 0; i < 5; i++) {
93+
log("starting compilation", "info");
94+
result = tsserver.compile("index.ts",
95+
`const buf = Buffer.alloc(${i} + 1);
96+
console.log(buf.length)`,
97+
);
98+
log("compilation done!", "info");
99+
expect(result.success).to.be.true;
100+
expect(result.declarations).to.equal("declare const buf: Buffer;\r\n");
101+
// call durations: ~1200, then ~100..200 ms for the following calls
102+
}
103+
});
105104
});

src/server.ts

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,15 +20,23 @@ export class Server {
2020

2121
// set default compiler options
2222
this.options = this.options || {};
23-
// TODO: We would like this to be true, but there's a bit performance hit
24-
/* if (this.options.noEmitOnError == null) */ this.options.noEmitOnError = false;
23+
this.options.moduleResolution = ts.ModuleResolutionKind.NodeJs;
24+
// Don't emit faulty code (by default)
25+
if (this.options.noEmitOnError == null) this.options.noEmitOnError = true;
2526
// emit declarations if possible
2627
if (this.options.declaration == null) this.options.declaration = true;
27-
this.options.moduleResolution = ts.ModuleResolutionKind.NodeJs;
28+
29+
// According to https://github.com/Microsoft/TypeScript/issues/24444#issuecomment-392970120
30+
// combining noEmitOnError=true and declaration=true massively increases the work done
31+
// by the compiler. To work around it, we call the compiler with noEmitOnError=false
32+
// and use the actual value to determine if we continue with the emit
33+
const internalOptions = Object.assign({}, this.options, {
34+
noEmitOnError: false,
35+
} as ts.CompilerOptions);
2836

2937
// set up the build pipeline
3038
this.fs = new VirtualFileSystem();
31-
this.host = new InMemoryServiceHost(this.fs, this.options);
39+
this.host = new InMemoryServiceHost(this.fs, internalOptions);
3240
this.service = ts.createLanguageService(this.host, ts.createDocumentRegistry());
3341

3442
// provide the requested lib files
@@ -98,8 +106,8 @@ ${type.toUpperCase()}: ${description}`;
98106

99107
const hasError = (
100108
(
101-
!diagnostics.every(d => d.type !== "error") ||
102-
(emitResult.emitSkipped && !this.options.emitDeclarationOnly)
109+
diagnostics.find(d => d.type === "error") != null
110+
|| (emitResult.emitSkipped && !this.options.emitDeclarationOnly)
103111
)
104112
&& this.options.noEmitOnError
105113
);

0 commit comments

Comments
 (0)