Skip to content
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

fix: type-check included files missed by transform (type-only files) #345

Merged
merged 8 commits into from
Jul 12, 2022
44 changes: 38 additions & 6 deletions __tests__/integration/errors.spec.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { jest, afterAll, test, expect } from "@jest/globals";
import { Mock } from "jest-mock"
import * as path from "path";
import { normalizePath as normalize } from "@rollup/pluginutils";
import * as fs from "fs-extra";
Expand All @@ -12,15 +13,14 @@ jest.setTimeout(15000);

const local = (x: string) => path.resolve(__dirname, x);
const cacheRoot = local("__temp/errors/rpt2-cache"); // don't use the one in node_modules
const onwarn = jest.fn();

afterAll(async () => {
// workaround: there seems to be some race condition causing fs.remove to fail, so give it a sec first (c.f. https://github.com/jprichardson/node-fs-extra/issues/532)
await new Promise(resolve => setTimeout(resolve, 1000));
await fs.remove(cacheRoot);
});

async function genBundle(relInput: string, extraOpts?: RPT2Options) {
async function genBundle(relInput: string, extraOpts?: RPT2Options, onwarn?: Mock) {
const input = normalize(local(`fixtures/errors/${relInput}`));
return helpers.genBundle({
input,
Expand All @@ -43,9 +43,10 @@ test("integration - semantic error", async () => {
});

test("integration - semantic error - abortOnError: false / check: false", async () => {
const onwarn = jest.fn();
// either warning or not type-checking should result in the same bundle
const { output } = await genBundle("semantic.ts", { abortOnError: false });
const { output: output2 } = await genBundle("semantic.ts", { check: false });
const { output } = await genBundle("semantic.ts", { abortOnError: false }, onwarn);
const { output: output2 } = await genBundle("semantic.ts", { check: false }, onwarn);
expect(output).toEqual(output2);

expect(output[0].fileName).toEqual("index.js");
Expand All @@ -60,7 +61,38 @@ test("integration - syntax error", () => {
});

test("integration - syntax error - abortOnError: false / check: false", () => {
const onwarn = jest.fn();
const err = "Unexpected token (Note that you need plugins to import files that are not JavaScript)";
expect(genBundle("syntax.ts", { abortOnError: false })).rejects.toThrow(err);
expect(genBundle("syntax.ts", { check: false })).rejects.toThrow(err);
expect(genBundle("syntax.ts", { abortOnError: false }, onwarn)).rejects.toThrow(err);
expect(genBundle("syntax.ts", { check: false }, onwarn)).rejects.toThrow(err);
});

const typeOnlyIncludes = ["**/import-type-error.ts", "**/type-only-import-with-error.ts"];

test("integration - type-only import error", () => {
expect(genBundle("import-type-error.ts", {
include: typeOnlyIncludes,
})).rejects.toThrow("Property 'nonexistent' does not exist on type 'someObj'.");
});

test("integration - type-only import error - abortOnError: false / check: false", async () => {
const onwarn = jest.fn();
// either warning or not type-checking should result in the same bundle
const { output } = await genBundle("import-type-error.ts", {
include: typeOnlyIncludes,
abortOnError: false,
}, onwarn);
const { output: output2 } = await genBundle("import-type-error.ts", {
include: typeOnlyIncludes,
check: false,
}, onwarn);
expect(output).toEqual(output2);

expect(output[0].fileName).toEqual("index.js");
expect(output[1].fileName).toEqual("import-type-error.d.ts");
expect(output[2].fileName).toEqual("import-type-error.d.ts.map");
expect(output[3].fileName).toEqual("type-only-import-with-error.d.ts");
expect(output[4].fileName).toEqual("type-only-import-with-error.d.ts.map");
expect(output.length).toEqual(5); // no other files
expect(onwarn).toBeCalledTimes(1);
});
8 changes: 8 additions & 0 deletions __tests__/integration/fixtures/errors/import-type-error.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
// this file has no errors itself; it is used an entry file to test an error in a type-only import

export type { typeError } from "./type-only-import-with-error";

// some code so this file isn't empty
export function sum(a: number, b: number) {
return a + b;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
type someObj = {};
export type typeError = someObj['nonexistent'];
78 changes: 49 additions & 29 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { relative, dirname, normalize as pathNormalize, resolve } from "path";
import * as tsTypes from "typescript";
import { PluginImpl, PluginContext, InputOptions, OutputOptions, TransformResult, SourceMap, Plugin } from "rollup";
import { PluginImpl, InputOptions, TransformResult, SourceMap, Plugin } from "rollup";
import { normalizePath as normalize } from "@rollup/pluginutils";
import { blue, red, yellow, green } from "colors/safe";
import findCacheDir from "find-cache-dir";
Expand Down Expand Up @@ -33,6 +33,7 @@ const typescript: PluginImpl<RPT2Options> = (options) =>
let service: tsTypes.LanguageService;
let noErrors = true;
const declarations: { [name: string]: { type: tsTypes.OutputFile; map?: tsTypes.OutputFile } } = {};
const checkedFiles = new Set<string>();

let _cache: TsCache;
const cache = (): TsCache =>
Expand All @@ -55,13 +56,24 @@ const typescript: PluginImpl<RPT2Options> = (options) =>

const typecheckFile = (id: string, snapshot: tsTypes.IScriptSnapshot, tcContext: IContext) =>
{
checkedFiles.add(id); // must come before print, as that could bail

const diagnostics = getDiagnostics(id, snapshot);
printDiagnostics(tcContext, diagnostics, parsedConfig.options.pretty !== false);

if (diagnostics.length > 0)
noErrors = false;
}

/** to be called at the end of Rollup's build phase, before output generation */
const buildDone = (): void =>
{
if (!watchMode && !noErrors)
context.info(yellow("there were errors or warnings."));

cache().done();
}

const pluginOptions: IOptions = Object.assign({},
{
check: true,
Expand All @@ -86,7 +98,7 @@ const typescript: PluginImpl<RPT2Options> = (options) =>
}
setTypescriptModule(pluginOptions.typescript);

const self: Plugin & { _ongenerate: () => void, _onwrite: (this: PluginContext, _output: OutputOptions) => void } = {
const self: Plugin = {

name: "rpt2",

Expand Down Expand Up @@ -141,6 +153,7 @@ const typescript: PluginImpl<RPT2Options> = (options) =>
{
const key = normalize(id);
delete declarations[key];
checkedFiles.delete(key);
},

resolveId(importee, importer)
Expand Down Expand Up @@ -200,9 +213,7 @@ const typescript: PluginImpl<RPT2Options> = (options) =>
{
// always checking on fatal errors, even if options.check is set to false
typecheckFile(id, snapshot, contextWrapper);

// since no output was generated, aborting compilation
cache().done();
this.error(red(`failed to transpile '${id}'`));
}

Expand Down Expand Up @@ -253,28 +264,22 @@ const typescript: PluginImpl<RPT2Options> = (options) =>

buildEnd(err)
{
if (!err)
return

// workaround: err.stack contains err.message and Rollup prints both, causing duplication, so split out the stack itself if it exists (c.f. https://github.com/ezolenko/rollup-plugin-typescript2/issues/103#issuecomment-1172820658)
const stackOnly = err.stack?.split(err.message)[1];
if (stackOnly)
this.error({ ...err, message: err.message, stack: stackOnly });
else
this.error(err);
},

generateBundle(bundleOptions)
{
self._ongenerate();
self._onwrite.call(this, bundleOptions);
},
if (err)
{
buildDone();
// workaround: err.stack contains err.message and Rollup prints both, causing duplication, so split out the stack itself if it exists (c.f. https://github.com/ezolenko/rollup-plugin-typescript2/issues/103#issuecomment-1172820658)
const stackOnly = err.stack?.split(err.message)[1];
if (stackOnly)
this.error({ ...err, message: err.message, stack: stackOnly });
else
this.error(err);
}

_ongenerate(): void
{
context.debug(() => `generating target ${generateRound + 1}`);
if (!pluginOptions.check)
return buildDone();

if (pluginOptions.check && watchMode && generateRound === 0)
// walkTree once on each cycle when in watch mode
if (watchMode)
{
cache().walkTree((id) =>
{
Expand All @@ -287,15 +292,30 @@ const typescript: PluginImpl<RPT2Options> = (options) =>
});
}

if (!watchMode && !noErrors)
context.info(yellow("there were errors or warnings."));
const contextWrapper = new RollupContext(pluginOptions.verbosity, pluginOptions.abortOnError, this, "rpt2: ");

cache().done();
generateRound++;
// type-check missed files as well
parsedConfig.fileNames.forEach((name) =>
{
const key = normalize(name);
if (checkedFiles.has(key) || !filter(key)) // don't duplicate if it's already been checked
return;

context.debug(() => `type-checking missed '${key}'`);

const snapshot = servicesHost.getScriptSnapshot(key);
if (snapshot)
typecheckFile(key, snapshot, contextWrapper);
});

buildDone();
},

_onwrite(this: PluginContext, _output: OutputOptions): void
generateBundle(this, _output)
{
context.debug(() => `generating target ${generateRound + 1}`);
generateRound++;

if (!parsedConfig.options.declaration)
return;

Expand Down