From d79a55931e99f56c2cca8df8eeb4679d3894be69 Mon Sep 17 00:00:00 2001 From: Anton Gilgur Date: Sat, 30 Jul 2022 16:05:12 -0400 Subject: [PATCH] clean: remove `ConsoleContext` entirely by using `buildStart` - the only reason that `ConsoleContext` was still used was because the `options` hook doesn't support certain context functions like `this.error` / `this.warn` etc - but `buildStart` does! so we can just move pretty much everything into `buildStart` instead - didn't move setting `rollupOptions` because that value gets hashed and the value in `buildStart` is different, as it is after all plugins' `options` hooks have ran - this way we don't need `ConsoleContext` what-so-ever and so can remove it entirely! - as well as `IContext`, its tests, etc - use `this.error` instead of `throw`ing errors in `parse-tsconfig` as it exists now - couldn't in the `options` hook, can in `buildStart` - this also ensure we don't have all the `rpt2: ` prefixes ever missing again - c.f. ff8895103c8466694e7d8eeb734f51d2850670d8, 0628482ea26ddf56e6ef5521e4dcb85ef5b4beb6 - refactor `parse-tsconfig.spec` to account for this change - c.f. Rollup hooks docs: https://rollupjs.org/guide/en/#options, https://rollupjs.org/guide/en/#buildstart --- __tests__/context.spec.ts | 46 +---------------------------- __tests__/fixtures/context.ts | 6 ++-- __tests__/parse-tsconfig.spec.ts | 38 +++++++++++++++++------- src/context.ts | 50 ++------------------------------ src/get-options-overrides.ts | 4 +-- src/index.ts | 27 +++++++++-------- src/parse-tsconfig.ts | 10 +++---- src/print-diagnostics.ts | 4 +-- src/tscache.ts | 4 +-- 9 files changed, 57 insertions(+), 132 deletions(-) diff --git a/__tests__/context.spec.ts b/__tests__/context.spec.ts index f2fc0da0..9a3603b9 100644 --- a/__tests__/context.spec.ts +++ b/__tests__/context.spec.ts @@ -1,7 +1,7 @@ import { jest, test, expect } from "@jest/globals"; import { makeContext } from "./fixtures/context"; -import { ConsoleContext, RollupContext } from "../src/context"; +import { RollupContext } from "../src/context"; (global as any).console = { warn: jest.fn(), @@ -9,50 +9,6 @@ import { ConsoleContext, RollupContext } from "../src/context"; info: jest.fn(), }; -test("ConsoleContext", () => { - const proxy = new ConsoleContext(6, "=>"); - - proxy.warn("test"); - expect(console.log).toHaveBeenLastCalledWith("=>test"); - - proxy.error("test2"); - expect(console.log).toHaveBeenLastCalledWith("=>test2"); - - proxy.info("test3"); - expect(console.log).toHaveBeenLastCalledWith("=>test3"); - - proxy.debug("test4"); - expect(console.log).toHaveBeenLastCalledWith("=>test4"); - - proxy.warn(() => "ftest"); - expect(console.log).toHaveBeenLastCalledWith("=>ftest"); - - proxy.error(() => "ftest2"); - expect(console.log).toHaveBeenLastCalledWith("=>ftest2"); - - proxy.info(() => "ftest3"); - expect(console.log).toHaveBeenLastCalledWith("=>ftest3"); - - proxy.debug(() => "ftest4"); - expect(console.log).toHaveBeenLastCalledWith("=>ftest4"); -}); - -test("ConsoleContext 0 verbosity", () => { - const proxy = new ConsoleContext(-100); - - proxy.warn("no-test"); - expect(console.log).not.toHaveBeenLastCalledWith("no-test"); - - proxy.info("no-test2"); - expect(console.log).not.toHaveBeenLastCalledWith("no-test2"); - - proxy.debug("no-test3"); - expect(console.log).not.toHaveBeenLastCalledWith("no-test3"); - - proxy.error("no-test4"); - expect(console.log).not.toHaveBeenLastCalledWith("no-test4"); -}); - test("RollupContext", () => { const innerContext = makeContext(); const context = new RollupContext(5, false, innerContext); diff --git a/__tests__/fixtures/context.ts b/__tests__/fixtures/context.ts index 239114c3..3031fad1 100644 --- a/__tests__/fixtures/context.ts +++ b/__tests__/fixtures/context.ts @@ -1,7 +1,7 @@ import { jest } from "@jest/globals"; import { PluginContext } from "rollup"; -import { IContext } from "../../src/context"; +import { RollupContext } from "../../src/context"; // if given a function, make sure to call it (for code coverage etc) function returnText (message: string | (() => string)) { @@ -11,11 +11,11 @@ function returnText (message: string | (() => string)) { return message(); } -export function makeContext(): PluginContext & IContext { +export function makeContext(): PluginContext & RollupContext { return { error: jest.fn(returnText), warn: jest.fn(returnText), info: jest.fn(returnText), debug: jest.fn(returnText), - } as unknown as PluginContext & IContext; + } as unknown as PluginContext & RollupContext; }; diff --git a/__tests__/parse-tsconfig.spec.ts b/__tests__/parse-tsconfig.spec.ts index 46a5a1c6..f19396e6 100644 --- a/__tests__/parse-tsconfig.spec.ts +++ b/__tests__/parse-tsconfig.spec.ts @@ -11,41 +11,57 @@ const local = (x: string) => normalize(path.resolve(__dirname, x)); const defaultOpts = makeOptions("", ""); test("parseTsConfig", () => { - expect(() => parseTsConfig(makeContext(), defaultOpts)).not.toThrow(); + const context = makeContext(); + + parseTsConfig(context, defaultOpts); + + expect(context.error).not.toBeCalled(); }); test("parseTsConfig - incompatible module", () => { - expect(() => parseTsConfig(makeContext(), { + const context = makeContext(); + + parseTsConfig(context, { ...defaultOpts, tsconfigOverride: { compilerOptions: { module: "none" } }, - })).toThrow("Incompatible tsconfig option. Module resolves to 'None'. This is incompatible with Rollup, please use"); + }); + + expect(context.error).toHaveBeenLastCalledWith(expect.stringContaining("Incompatible tsconfig option. Module resolves to 'None'. This is incompatible with Rollup, please use")); }); test("parseTsConfig - tsconfig errors", () => { const context = makeContext(); - // should not throw when the tsconfig is buggy, but should still print an error (below) - expect(() => parseTsConfig(context, { + parseTsConfig(context, { ...defaultOpts, tsconfigOverride: { include: "should-be-an-array", }, - })).not.toThrow(); + }); + expect(context.error).toHaveBeenLastCalledWith(expect.stringContaining("Compiler option 'include' requires a value of type Array")); }); test("parseTsConfig - failed to open", () => { - expect(() => parseTsConfig(makeContext(), { + const context = makeContext(); + const nonExistentTsConfig = "non-existent-tsconfig"; + + parseTsConfig(context, { ...defaultOpts, - tsconfig: "non-existent-tsconfig", - })).toThrow("rpt2: failed to open 'non-existent-tsconfig'"); + tsconfig: nonExistentTsConfig, + }) + + expect(context.error).toHaveBeenLastCalledWith(expect.stringContaining(`failed to open '${nonExistentTsConfig}`)); }); test("parseTsConfig - failed to parse", () => { + const context = makeContext(); const notTsConfigPath = local("fixtures/options.ts"); // a TS file should fail to parse - expect(() => parseTsConfig(makeContext(), { + parseTsConfig(context, { ...defaultOpts, tsconfig: notTsConfigPath, - })).toThrow(`rpt2: failed to parse '${notTsConfigPath}'`); + }) + + expect(context.error).toHaveBeenLastCalledWith(expect.stringContaining(`failed to parse '${notTsConfigPath}'`)); }); diff --git a/src/context.ts b/src/context.ts index 50b994a0..83c2e7c7 100644 --- a/src/context.ts +++ b/src/context.ts @@ -1,13 +1,5 @@ import { PluginContext } from "rollup"; -export interface IContext -{ - warn(message: string | (() => string)): void; - error(message: string | (() => string)): void; - info(message: string | (() => string)): void; - debug(message: string | (() => string)): void; -} - export enum VerbosityLevel { Error = 0, @@ -20,46 +12,8 @@ function getText (message: string | (() => string)): string { return typeof message === "string" ? message : message(); } -/* tslint:disable:max-classes-per-file -- generally a good rule to follow, but these two classes could basically be one */ - -/** mainly to be used in options hook, but can be used in other hooks too */ -export class ConsoleContext implements IContext -{ - constructor(private verbosity: VerbosityLevel, private prefix: string = "") - { - } - - public warn(message: string | (() => string)): void - { - if (this.verbosity < VerbosityLevel.Warning) - return; - console.log(`${this.prefix}${getText(message)}`); - } - - public error(message: string | (() => string)): void - { - if (this.verbosity < VerbosityLevel.Error) - return; - console.log(`${this.prefix}${getText(message)}`); - } - - public info(message: string | (() => string)): void - { - if (this.verbosity < VerbosityLevel.Info) - return; - console.log(`${this.prefix}${getText(message)}`); - } - - public debug(message: string | (() => string)): void - { - if (this.verbosity < VerbosityLevel.Debug) - return; - console.log(`${this.prefix}${getText(message)}`); - } -} - /** cannot be used in options hook (which does not have this.warn and this.error), but can be in other hooks */ -export class RollupContext implements IContext +export class RollupContext { constructor(private verbosity: VerbosityLevel, private bail: boolean, private context: PluginContext, private prefix: string = "") { @@ -72,7 +26,7 @@ export class RollupContext implements IContext this.context.warn(`${getText(message)}`); } - public error(message: string | (() => string)): void + public error(message: string | (() => string)): void | never { if (this.verbosity < VerbosityLevel.Error) return; diff --git a/src/get-options-overrides.ts b/src/get-options-overrides.ts index 9634122f..f4c756ff 100644 --- a/src/get-options-overrides.ts +++ b/src/get-options-overrides.ts @@ -4,7 +4,7 @@ import { createFilter as createRollupFilter, normalizePath as normalize } from " import { tsModule } from "./tsproxy"; import { IOptions } from "./ioptions"; -import { IContext } from "./context"; +import { RollupContext } from "./context"; export function getOptionsOverrides({ useTsconfigDeclarationDir, cacheRoot }: IOptions, preParsedTsconfig?: tsTypes.ParsedCommandLine): tsTypes.CompilerOptions { @@ -51,7 +51,7 @@ function expandIncludeWithDirs(include: string | string[], dirs: string[]) return newDirs; } -export function createFilter(context: IContext, pluginOptions: IOptions, parsedConfig: tsTypes.ParsedCommandLine) +export function createFilter(context: RollupContext, pluginOptions: IOptions, parsedConfig: tsTypes.ParsedCommandLine) { let included = pluginOptions.include; let excluded = pluginOptions.exclude; diff --git a/src/index.ts b/src/index.ts index 14984e85..0b1ca2b8 100644 --- a/src/index.ts +++ b/src/index.ts @@ -5,7 +5,7 @@ import { normalizePath as normalize } from "@rollup/pluginutils"; import { blue, red, yellow, green } from "colors/safe"; import findCacheDir from "find-cache-dir"; -import { ConsoleContext, RollupContext, IContext, VerbosityLevel } from "./context"; +import { RollupContext, VerbosityLevel } from "./context"; import { LanguageServiceHost } from "./host"; import { TsCache, convertDiagnostic, convertEmitOutput, getAllReferences, ICode } from "./tscache"; import { tsModule, setTypescriptModule } from "./tsproxy"; @@ -24,7 +24,7 @@ const typescript: PluginImpl = (options) => let watchMode = false; let generateRound = 0; let rollupOptions: InputOptions; - let context: ConsoleContext; + let context: RollupContext; let filter: any; let parsedConfig: tsTypes.ParsedCommandLine; let tsConfigPath: string | undefined; @@ -54,7 +54,7 @@ const typescript: PluginImpl = (options) => })); } - const typecheckFile = (id: string, snapshot: tsTypes.IScriptSnapshot | undefined, tcContext: IContext) => + const typecheckFile = (id: string, snapshot: tsTypes.IScriptSnapshot | undefined, tcContext: RollupContext) => { if (!snapshot) return; @@ -119,8 +119,13 @@ const typescript: PluginImpl = (options) => options(config) { - rollupOptions = {... config}; - context = new ConsoleContext(pluginOptions.verbosity, "rpt2: "); + rollupOptions = { ...config }; + return config; + }, + + buildStart() + { + context = new RollupContext(pluginOptions.verbosity, pluginOptions.abortOnError, this, "rpt2: "); watchMode = process.env.ROLLUP_WATCH === "true" || !!this.meta.watchMode; // meta.watchMode was added in 2.14.0 to capture watch via Rollup API (i.e. no env var) (c.f. https://github.com/rollup/rollup/blob/master/CHANGELOG.md#2140) ({ parsedTsConfig: parsedConfig, fileName: tsConfigPath } = parseTsConfig(context, pluginOptions)); @@ -157,8 +162,6 @@ const typescript: PluginImpl = (options) => if (diagnostics.length > 0) noErrors = false; } - - return config; }, watchChange(id) @@ -210,8 +213,6 @@ const typescript: PluginImpl = (options) => if (!filter(id)) return undefined; - const contextWrapper = new RollupContext(pluginOptions.verbosity, pluginOptions.abortOnError, this, "rpt2: "); - const snapshot = servicesHost.setSnapshot(id, code); // getting compiled file from cache or from ts @@ -223,7 +224,7 @@ const typescript: PluginImpl = (options) => { noErrors = false; // always checking on fatal errors, even if options.check is set to false - typecheckFile(id, snapshot, contextWrapper); + typecheckFile(id, snapshot, context); // since no output was generated, aborting compilation this.error(red(`Emit skipped for '${id}'. See https://github.com/microsoft/TypeScript/issues/49790 for potential reasons why this may occur`)); } @@ -233,7 +234,7 @@ const typescript: PluginImpl = (options) => }); if (pluginOptions.check) - typecheckFile(id, snapshot, contextWrapper); + typecheckFile(id, snapshot, context); if (!result) return undefined; @@ -299,8 +300,6 @@ const typescript: PluginImpl = (options) => }); } - const contextWrapper = new RollupContext(pluginOptions.verbosity, pluginOptions.abortOnError, this, "rpt2: "); - // type-check missed files as well parsedConfig.fileNames.forEach((name) => { @@ -310,7 +309,7 @@ const typescript: PluginImpl = (options) => context.debug(() => `type-checking missed '${key}'`); const snapshot = servicesHost.getScriptSnapshot(key); - typecheckFile(key, snapshot, contextWrapper); + typecheckFile(key, snapshot, context); }); buildDone(); diff --git a/src/parse-tsconfig.ts b/src/parse-tsconfig.ts index dd41bffe..740938be 100644 --- a/src/parse-tsconfig.ts +++ b/src/parse-tsconfig.ts @@ -2,19 +2,19 @@ import { dirname } from "path"; import * as _ from "lodash"; import { tsModule } from "./tsproxy"; -import { IContext } from "./context"; +import { RollupContext } from "./context"; import { printDiagnostics } from "./print-diagnostics"; import { convertDiagnostic } from "./tscache"; import { getOptionsOverrides } from "./get-options-overrides"; import { IOptions } from "./ioptions"; -export function parseTsConfig(context: IContext, pluginOptions: IOptions) +export function parseTsConfig(context: RollupContext, pluginOptions: IOptions) { const fileName = tsModule.findConfigFile(pluginOptions.cwd, tsModule.sys.fileExists, pluginOptions.tsconfig); // if the value was provided, but no file, fail hard if (pluginOptions.tsconfig !== undefined && !fileName) - throw new Error(`rpt2: failed to open '${pluginOptions.tsconfig}'`); + context.error(`failed to open '${pluginOptions.tsconfig}'`); let loadedConfig: any = {}; let baseDir = pluginOptions.cwd; @@ -29,7 +29,7 @@ export function parseTsConfig(context: IContext, pluginOptions: IOptions) if (result.error !== undefined) { printDiagnostics(context, convertDiagnostic("config", [result.error]), pretty); - throw new Error(`rpt2: failed to parse '${fileName}'`); + context.error(`failed to parse '${fileName}'`); } loadedConfig = result.config; @@ -46,7 +46,7 @@ export function parseTsConfig(context: IContext, pluginOptions: IOptions) const module = parsedTsConfig.options.module!; if (module !== tsModule.ModuleKind.ES2015 && module !== tsModule.ModuleKind.ES2020 && module !== tsModule.ModuleKind.ESNext) - throw new Error(`rpt2: Incompatible tsconfig option. Module resolves to '${tsModule.ModuleKind[module]}'. This is incompatible with Rollup, please use 'module: "ES2015"', 'module: "ES2020"', or 'module: "ESNext"'.`); + context.error(`Incompatible tsconfig option. Module resolves to '${tsModule.ModuleKind[module]}'. This is incompatible with Rollup, please use 'module: "ES2015"', 'module: "ES2020"', or 'module: "ESNext"'.`); printDiagnostics(context, convertDiagnostic("config", parsedTsConfig.errors), pretty); diff --git a/src/print-diagnostics.ts b/src/print-diagnostics.ts index ef49895f..868c09e0 100644 --- a/src/print-diagnostics.ts +++ b/src/print-diagnostics.ts @@ -1,10 +1,10 @@ import { red, white, yellow } from "colors/safe"; import { tsModule } from "./tsproxy"; -import { IContext } from "./context"; +import { RollupContext } from "./context"; import { IDiagnostics } from "./tscache"; -export function printDiagnostics(context: IContext, diagnostics: IDiagnostics[], pretty = true): void +export function printDiagnostics(context: RollupContext, diagnostics: IDiagnostics[], pretty = true): void { diagnostics.forEach((diagnostic) => { diff --git a/src/tscache.ts b/src/tscache.ts index 71614a5d..9fabd21a 100644 --- a/src/tscache.ts +++ b/src/tscache.ts @@ -5,7 +5,7 @@ import { Graph, alg } from "graphlib"; import objHash from "object-hash"; import { blue, yellow, green } from "colors/safe"; -import { IContext } from "./context"; +import { RollupContext } from "./context"; import { RollingCache } from "./rollingcache"; import { ICache } from "./icache"; import { tsModule } from "./tsproxy"; @@ -111,7 +111,7 @@ export class TsCache private syntacticDiagnosticsCache!: ICache; private hashOptions = { algorithm: "sha1", ignoreUnknown: false }; - constructor(private noCache: boolean, hashIgnoreUnknown: boolean, private host: tsTypes.LanguageServiceHost, private cacheRoot: string, private options: tsTypes.CompilerOptions, private rollupConfig: any, rootFilenames: string[], private context: IContext) + constructor(private noCache: boolean, hashIgnoreUnknown: boolean, private host: tsTypes.LanguageServiceHost, private cacheRoot: string, private options: tsTypes.CompilerOptions, private rollupConfig: any, rootFilenames: string[], private context: RollupContext) { this.dependencyTree = new Graph({ directed: true }); this.dependencyTree.setDefaultNodeLabel((_node: string) => ({ dirty: false }));