From 962b5e5c70578ed7efe9fa6b2d0d267e08b047e9 Mon Sep 17 00:00:00 2001 From: Sam Brown Date: Tue, 28 Mar 2023 14:27:51 +0100 Subject: [PATCH] Implement selective typecheck (#2321) * Implement selective options for typecheck, apply selective logic to includes * Segregate modularRoot util for tests * Further tidy up of new command testing approach * Add fixture and new test cases for selective typecheck * Add changeset * Cover allowlisted properties, fix path bug in util --- .changeset/tiny-parents-invent.md | 5 + .../selective-typecheck-example/package.json | 60 ++++ .../packages/common-module/package.json | 10 + .../packages/common-module/src/index.ts | 12 + .../packages/esbuild-app/.modular.js | 3 + .../packages/esbuild-app/package.json | 11 + .../packages/esbuild-app/src/App.tsx | 24 ++ .../packages/webpack-app/package.json | 11 + .../packages/webpack-app/src/App.tsx | 24 ++ docs/commands/typecheck.md | 54 +++- .../src/__tests__/typecheck.test.ts | 258 ++++++++++++++---- packages/modular-scripts/src/program.ts | 50 ++-- packages/modular-scripts/src/test/utils.ts | 61 ++++- packages/modular-scripts/src/typecheck.ts | 116 ++++++-- .../src/utils/getPackageMetadata.ts | 5 +- .../src/utils/validateOptions.ts | 16 ++ 16 files changed, 618 insertions(+), 102 deletions(-) create mode 100644 .changeset/tiny-parents-invent.md create mode 100644 __fixtures__/selective-typecheck-example/package.json create mode 100644 __fixtures__/selective-typecheck-example/packages/common-module/package.json create mode 100644 __fixtures__/selective-typecheck-example/packages/common-module/src/index.ts create mode 100644 __fixtures__/selective-typecheck-example/packages/esbuild-app/.modular.js create mode 100644 __fixtures__/selective-typecheck-example/packages/esbuild-app/package.json create mode 100644 __fixtures__/selective-typecheck-example/packages/esbuild-app/src/App.tsx create mode 100644 __fixtures__/selective-typecheck-example/packages/webpack-app/package.json create mode 100644 __fixtures__/selective-typecheck-example/packages/webpack-app/src/App.tsx create mode 100644 packages/modular-scripts/src/utils/validateOptions.ts diff --git a/.changeset/tiny-parents-invent.md b/.changeset/tiny-parents-invent.md new file mode 100644 index 000000000..2837b9a16 --- /dev/null +++ b/.changeset/tiny-parents-invent.md @@ -0,0 +1,5 @@ +--- +'modular-scripts': minor +--- + +Add selective options to typecheck diff --git a/__fixtures__/selective-typecheck-example/package.json b/__fixtures__/selective-typecheck-example/package.json new file mode 100644 index 000000000..771a6acf0 --- /dev/null +++ b/__fixtures__/selective-typecheck-example/package.json @@ -0,0 +1,60 @@ +{ + "name": "selective-typecheck-example", + "version": "1.0.0", + "main": "index.js", + "author": "Sam Brown ", + "license": "MIT", + "private": true, + "workspaces": [ + "packages/**" + ], + "modular": { + "type": "root" + }, + "scripts": { + "start": "modular start", + "build": "modular build", + "test": "modular test", + "lint": "eslint . --ext .js,.ts,.tsx", + "prettier": "prettier --write ." + }, + "eslintConfig": { + "extends": "modular-app" + }, + "browserslist": { + "production": [ + ">0.2%", + "not dead", + "not op_mini all" + ], + "development": [ + "last 1 chrome version", + "last 1 firefox version", + "last 1 safari version" + ] + }, + "prettier": { + "singleQuote": true, + "trailingComma": "all", + "printWidth": 80, + "proseWrap": "always" + }, + "dependencies": { + "@testing-library/dom": "^8.20.0", + "@testing-library/jest-dom": "^5.16.5", + "@testing-library/react": "^9.5.0", + "@testing-library/user-event": "^7.2.1", + "@types/jest": "^29.4.0", + "@types/node": "^18.14.6", + "@types/react": "^18.0.9", + "@types/react-dom": "^18.0.8", + "eslint-config-modular-app": "^4.0.0", + "modular-scripts": "^4.1.0", + "modular-template-app": "^1.2.0", + "modular-template-source": "^1.1.0", + "prettier": "^2.8.4", + "react": "^18.2.0", + "react-dom": "^18.2.0", + "typescript": "^4.8.3" + } +} diff --git a/__fixtures__/selective-typecheck-example/packages/common-module/package.json b/__fixtures__/selective-typecheck-example/packages/common-module/package.json new file mode 100644 index 000000000..211a26449 --- /dev/null +++ b/__fixtures__/selective-typecheck-example/packages/common-module/package.json @@ -0,0 +1,10 @@ +{ + "name": "selective-typecheck-common-module", + "private": false, + "modular": { + "type": "source" + }, + "main": "./src/index.ts", + "types": "./src/index.ts", + "version": "1.0.0" +} diff --git a/__fixtures__/selective-typecheck-example/packages/common-module/src/index.ts b/__fixtures__/selective-typecheck-example/packages/common-module/src/index.ts new file mode 100644 index 000000000..b0d6ba7f4 --- /dev/null +++ b/__fixtures__/selective-typecheck-example/packages/common-module/src/index.ts @@ -0,0 +1,12 @@ +/* eslint-disable */ + +// The following TS nocheck flag gets removed in test +// @ts-nocheck + +export default function add(a: number, b: number): number { + return a + b + 'c'; +} + +export function otherThing(input) { + return typeof input; +} diff --git a/__fixtures__/selective-typecheck-example/packages/esbuild-app/.modular.js b/__fixtures__/selective-typecheck-example/packages/esbuild-app/.modular.js new file mode 100644 index 000000000..6ae251fac --- /dev/null +++ b/__fixtures__/selective-typecheck-example/packages/esbuild-app/.modular.js @@ -0,0 +1,3 @@ +module.exports = { + useModularEsbuild: true, +}; diff --git a/__fixtures__/selective-typecheck-example/packages/esbuild-app/package.json b/__fixtures__/selective-typecheck-example/packages/esbuild-app/package.json new file mode 100644 index 000000000..7f8198ea9 --- /dev/null +++ b/__fixtures__/selective-typecheck-example/packages/esbuild-app/package.json @@ -0,0 +1,11 @@ +{ + "name": "app", + "private": true, + "modular": { + "type": "app" + }, + "version": "1.0.0", + "dependencies": { + "selective-typecheck-common-module": "*" + } +} diff --git a/__fixtures__/selective-typecheck-example/packages/esbuild-app/src/App.tsx b/__fixtures__/selective-typecheck-example/packages/esbuild-app/src/App.tsx new file mode 100644 index 000000000..c7d8589ff --- /dev/null +++ b/__fixtures__/selective-typecheck-example/packages/esbuild-app/src/App.tsx @@ -0,0 +1,24 @@ +import React, { useEffect } from 'react'; +import './App.css'; + +function App(): JSX.Element { + return ( +
+
+

+ Edit src/App.tsx and save to reload. +

+ + Learn React + +
+
+ ); +} + +export default App; diff --git a/__fixtures__/selective-typecheck-example/packages/webpack-app/package.json b/__fixtures__/selective-typecheck-example/packages/webpack-app/package.json new file mode 100644 index 000000000..32e1d2fd8 --- /dev/null +++ b/__fixtures__/selective-typecheck-example/packages/webpack-app/package.json @@ -0,0 +1,11 @@ +{ + "name": "webpack-app", + "private": true, + "modular": { + "type": "app" + }, + "version": "1.0.0", + "dependencies": { + "selective-typecheck-common-module": "*" + } +} diff --git a/__fixtures__/selective-typecheck-example/packages/webpack-app/src/App.tsx b/__fixtures__/selective-typecheck-example/packages/webpack-app/src/App.tsx new file mode 100644 index 000000000..a9184f7a1 --- /dev/null +++ b/__fixtures__/selective-typecheck-example/packages/webpack-app/src/App.tsx @@ -0,0 +1,24 @@ +import * as React from 'react'; +import './App.css'; + +function App(): JSX.Element { + return ( +
+
+

+ Edit src/App.tsx and save to reload. +

+ + Learn React + +
+
+ ); +} + +export default App; diff --git a/docs/commands/typecheck.md b/docs/commands/typecheck.md index c579c7171..c3fa8173c 100644 --- a/docs/commands/typecheck.md +++ b/docs/commands/typecheck.md @@ -3,16 +3,66 @@ parent: Commands title: modular typecheck --- -# `modular typecheck [options]` +# `modular typecheck [options] [packages...]` `modular typecheck` will programmatically report the semantic, syntactic, and -declaration type errors found in your code, based on your tsconfig.json. +declaration type errors found in your code, based on your `tsconfig.json`. In a CI environment, it will print condensed errors if they are present. In non-CI environments, it will print the full details of the error, line, and small snapshot of the offending line in question. +## Configuration + +`modular typecheck` will respect the root `tsconfig.json`, but most +`compilerOptions` are ignored. + +It should be noted that `modular typecheck` does not support package-level +`tsconfig.json` files, for backwards compatibility. Your IDE may still consume +these, but it is recommended to keep your TypeScript configuration in the root +`tsconfig.json`. + +_Why does Modular restrict compilerOptions?_ + +`modular typecheck` aims to verify that the project's types are passing without +errors. Certain TypeScript features such as emitting output are not useful in +this scenario, so Modular has always applied a restricted set of +`compilerOptions`. + +Although this approach has limited flexibility, it fits with Modular's goals and +brings certain advantages: + +- Configuration is kept minimal and simple where possible +- The possibility of incompatible or conflicting `compilerOptions` between + packages is avoided +- It enables the use of selective `modular typecheck` (i.e. supply package names + and using flags such as `--ancestors`) + +There are certain exceptions for practical use cases. The current allowlist is: + +- [jsx](https://www.typescriptlang.org/tsconfig#jsx) + +Some use cases may warrant new exceptions. If this is you, please file an issue +with the project for consideration. + ## Options: `--verbose`: Enables verbose logging within modular + +`--descendants`: Typecheck the packages specified by the `[packages...]` +argument and/or the `--changed` option and additionally typecheck all their +descendants (i.e. the packages they directly or indirectly depend on). + +`--ancestors`: Typecheck the packages specified by the `[packages...]` argument +and/or the `--changed` option and additionally typecheck all their ancestors +(i.e. the packages that have a direct or indirect dependency on them). + +`--changed`: Typecheck only the packages whose workspaces contain files that +have changed. Files that have changed are calculated comparing the current state +of the repository with the branch specified by `compareBranch` or, if +`compareBranch` is not set, with the default git branch. + +`--compareBranch`: Specify the comparison branch used to determine which files +have changed when using the `changed` option. If this option is used without +`changed`, the command will fail. diff --git a/packages/modular-scripts/src/__tests__/typecheck.test.ts b/packages/modular-scripts/src/__tests__/typecheck.test.ts index 9f90f4cf4..2f7084bd8 100644 --- a/packages/modular-scripts/src/__tests__/typecheck.test.ts +++ b/packages/modular-scripts/src/__tests__/typecheck.test.ts @@ -1,94 +1,244 @@ import * as path from 'path'; import * as fs from 'fs-extra'; -import execa from 'execa'; -import { createModularTestContext, runYarnModular } from '../test/utils'; -import getModularRoot from '../utils/getModularRoot'; +import { + createModularTestContext, + getRealModularRootInTest, + mockPreflightImplementation, +} from '../test/utils'; -const modularRoot = getModularRoot(); -const fixturesFolder = path.join(__dirname, '__fixtures__', 'typecheck'); -const relativeFixturePath = fixturesFolder.replace(modularRoot, ''); +const modularRoot = getRealModularRootInTest(); + +// Skip preflight in tests (faster, avoids the need to mock getModularRoot statically) +jest.mock('../utils/actionPreflightCheck', () => mockPreflightImplementation); describe('Modular typecheck', () => { - describe('when there are type errors', () => { + describe('not using selective options', () => { let tempModularRepo: string; let tempFixturesFolder: string; + beforeEach(() => { + /** + * - Set up a temp modular repo + * - Copy the `typecheck` fixture into it + * - Remove a `ts-nocheck` statement so that errors will happen as expected + */ tempModularRepo = createModularTestContext(); - tempFixturesFolder = path.join(tempModularRepo, relativeFixturePath); + tempFixturesFolder = path.join(tempModularRepo, 'packages', 'app', 'src'); fs.mkdirsSync(tempFixturesFolder); + const fixturesFolder = path.join(__dirname, '__fixtures__', 'typecheck'); + const invalidTsContent = fs + .readFileSync(path.join(fixturesFolder, 'InvalidTyping.ts')) + .toString() + .replaceAll('@ts-nocheck', ''); fs.writeFileSync( path.join(tempFixturesFolder, 'InvalidTyping.ts'), - fs - .readFileSync(path.join(fixturesFolder, 'InvalidTyping.ts'), 'utf-8') - .replace('//@ts-nocheck', '//'), + invalidTsContent, ); fs.copyFileSync( - path.join(modularRoot, 'packages', 'modular-scripts', 'tsconfig.json'), + path.join(modularRoot, 'tsconfig.json'), path.join(tempModularRepo, 'tsconfig.json'), ); + + // Mock the modular root per temporary modular repo + jest.doMock('../utils/getModularRoot', () => { + return { + __esModule: true, + default: () => tempModularRepo, + }; + }); }); describe('when in CI', () => { beforeEach(() => { process.env.CI = 'true'; }); + afterEach(() => { process.env.CI = undefined; }); + it('should display truncated errors', async () => { - let tsc = ''; + const { default: typecheck } = await import('../typecheck'); + let caughtError: Error | undefined; + const expectedErrorText = [ + "Cannot find module 'foo' or its corresponding type declarations", + "A function whose declared type is neither 'void' nor 'any' must return a value.", + ]; try { - await execa('tsc', ['--noEmit', '--pretty', 'false'], { - all: true, - cleanup: true, - cwd: tempModularRepo, - }); - } catch ({ stdout }) { - tsc = stdout as string; + await typecheck({}, []); + } catch (e) { + caughtError = e as Error; + } finally { + expect(caughtError).toBeTruthy(); + for (const msg of expectedErrorText) { + expect(caughtError?.message.includes(msg)).toBe(true); + } } - let modularStdErr = ''; - try { - await runYarnModular(tempModularRepo, 'typecheck'); - } catch ({ stderr }) { - modularStdErr = stderr as string; - } - const tscErrors = tsc.split('\n'); - const modularErrors = modularStdErr.split('\n'); - tscErrors.forEach((errorMessage: string, i: number) => { - expect(modularErrors[i]).toMatch(errorMessage); - }); }); }); + describe('when not in CI', () => { it('should match display full error logs', async () => { - let tsc = ''; - try { - await execa('tsc', ['--noEmit'], { - all: true, - cleanup: true, - cwd: tempModularRepo, - }); - } catch ({ stdout }) { - tsc = stdout as string; - } - let modularStdErr = ''; + const { default: typecheck } = await import('../typecheck'); + let caughtError: Error | undefined; + const expectedErrorText = [ + "Cannot find module 'foo' or its corresponding type declarations", + "A function whose declared type is neither 'void' nor 'any' must return a value.", + ]; try { - await runYarnModular(tempModularRepo, 'typecheck'); - } catch ({ stderr }) { - modularStdErr = stderr as string; + await typecheck({}, []); + } catch (e) { + caughtError = e as Error; + } finally { + expect(caughtError).toBeTruthy(); + for (const msg of expectedErrorText) { + expect(caughtError?.message.includes(msg)).toBe(true); + } } - const tscErrors = tsc.split('\n'); - const modularErrors = modularStdErr.split('\n'); - tscErrors.forEach((errorMessage: string, i: number) => { - expect(modularErrors[i]).toMatch(errorMessage); - }); }); }); }); - describe('when there are no type errors', () => { - it('should print a one line success message', async () => { - const result = await runYarnModular(modularRoot, 'typecheck'); - expect(result.stdout).toMatch('\u2713 Typecheck passed'); + + describe('using selective options', () => { + let tempModularRepo: string; + + beforeEach(() => { + // Required to achieve consistency when using `jest.doMock` in tests + jest.resetModules(); + + /** + * - Set up a temp modular repo + * - Copy the `selective-typecheck-example` fixture into it (3 packages within) + * - Remove a `ts-nocheck` statement so that errors will happen as expected + */ + tempModularRepo = createModularTestContext(); + fs.copySync( + path.join( + modularRoot, + '__fixtures__', + 'selective-typecheck-example', + 'packages', + ), + path.join(tempModularRepo, 'packages'), + ); + const invalidTsContent = fs + .readFileSync( + path.join( + modularRoot, + '__fixtures__', + 'selective-typecheck-example', + 'packages', + 'common-module', + 'src', + 'index.ts', + ), + ) + .toString() + .replaceAll('@ts-nocheck', ''); + fs.writeFileSync( + path.join( + tempModularRepo, + 'packages', + 'common-module', + 'src', + 'index.ts', + ), + invalidTsContent, + ); + + // Mock the modular root per temporary modular repo + jest.doMock('../utils/getModularRoot', () => { + return { + __esModule: true, + default: () => tempModularRepo, + }; + }); + }); + + it('should test descendants and throw', async () => { + const { default: typecheck } = await import('../typecheck'); + let caughtError: Error | undefined; + const expectedErrorText = [ + "Type 'string' is not assignable to type 'number'.", + "Parameter 'input' implicitly has an 'any' type.", + ]; + try { + await typecheck({ descendants: true }, ['webpack-app']); + } catch (e) { + caughtError = e as Error; + } finally { + expect(caughtError).toBeTruthy(); + for (const msg of expectedErrorText) { + expect(caughtError?.message.includes(msg)).toBe(true); + } + } + }); + + it('should not test descendants and not error', async () => { + const { default: typecheck } = await import('../typecheck'); + let caughtError: Error | undefined; + try { + await typecheck({ descendants: false }, ['webpack-app']); + } catch (e) { + caughtError = e as Error; + } finally { + expect(caughtError).toBeUndefined(); + } + }); + }); + + describe('using allowlisted compiler options', () => { + let tempModularRepo: string; + + beforeEach(() => { + // Required to achieve consistency when using `jest.doMock` in tests + jest.resetModules(); + + /** + * - Set up a temp modular repo + * - Copy the `selective-typecheck-example` fixture into it (3 packages within) + * - Create a custom `tsconfig.json` that uses allowlisted compiler options + */ + tempModularRepo = createModularTestContext(); + fs.copySync( + path.join( + modularRoot, + '__fixtures__', + 'selective-typecheck-example', + 'packages', + ), + path.join(tempModularRepo, 'packages'), + ); + // eslint-disable-next-line + const tsconfigContent: Record = JSON.parse( + fs.readFileSync(path.join(modularRoot, 'tsconfig.json')).toString(), + ); + const updatedTsConfig = { + ...tsconfigContent, + include: ['packages/**/src'], + compilerOptions: { + jsx: 'preserve', + }, + }; + fs.writeFileSync( + path.join(tempModularRepo, 'tsconfig.json'), + JSON.stringify(updatedTsConfig), + ); + + // Mock the modular root per temporary modular repo + jest.doMock('../utils/getModularRoot', () => { + return { + __esModule: true, + default: () => tempModularRepo, + }; + }); + }); + + it('accepts a non-default ("preserve") value for `jsx`', async () => { + // This option has no effect on typecheck, since typecheck doesn't emit. + // So, we just check that typecheck doesn't throw. + const { default: typecheck } = await import('../typecheck'); + await expect(typecheck({}, [])).resolves.not.toThrow(); }); }); }); diff --git a/packages/modular-scripts/src/program.ts b/packages/modular-scripts/src/program.ts index 9ef5a9493..ce5496d3d 100755 --- a/packages/modular-scripts/src/program.ts +++ b/packages/modular-scripts/src/program.ts @@ -4,13 +4,15 @@ import * as fs from 'fs-extra'; import isCI from 'is-ci'; import chalk from 'chalk'; import commander, { Option } from 'commander'; -import type { JSONSchemaForNPMPackageJsonFiles as PackageJson } from '@schemastore/package'; -import type { TestOptions } from './test'; -import type { LintOptions } from './lint'; import { testOptions } from './test/jestOptions'; - import actionPreflightCheck from './utils/actionPreflightCheck'; import * as logger from './utils/logger'; +import { validateCompareOptions } from './utils/validateOptions'; + +import type { JSONSchemaForNPMPackageJsonFiles as PackageJson } from '@schemastore/package'; +import type { TestOptions } from './test'; +import type { LintOptions } from './lint'; +import type { TypecheckOptions } from './typecheck'; const program = new commander.Command('modular'); program.version( @@ -124,12 +126,7 @@ program ) => { const { default: build } = await import('./build-scripts'); - if (options.compareBranch && !options.changed) { - process.stderr.write( - "Option --compareBranch doesn't make sense without option --changed\n", - ); - process.exit(1); - } + validateCompareOptions(options.compareBranch, options.changed); if (options.dangerouslyIgnoreCircularDependencies) { // Warn. Users should never use this, but if they use it, they should have cycles limited to "source" packages @@ -212,12 +209,7 @@ program .allowUnknownOption() .description('Run tests over the codebase') .action(async (packages: string[], options: CLITestOptions) => { - if (options.compareBranch && !options.changed) { - process.stderr.write( - "Option --compareBranch doesn't make sense without option --changed\n", - ); - process.exit(1); - } + validateCompareOptions(options.compareBranch, options.changed); const { default: test } = await import('./test'); @@ -289,12 +281,32 @@ program }); program - .command('typecheck') + .command('typecheck [packages...]') .description('Typechecks the entire project') .option('--verbose', 'Enables verbose logging within modular.') - .action(async () => { + .option( + '--ancestors', + 'Additionally run typecheck for workspaces that depend on workspaces that have changed', + false, + ) + .option( + '--descendants', + 'Additionally run typecheck for workspaces that directly or indirectly depend on the specified packages (can be combined with --changed)', + false, + ) + .option( + '--changed', + 'Run typecheck only for workspaces that have changed compared to the branch specified in --compareBranch', + ) + .option( + '--compareBranch ', + "Specifies the branch to use with the --changed flag. If not specified, Modular will use the repo's default branch", + ) + .action(async (packages: string[], options: TypecheckOptions) => { + validateCompareOptions(options.compareBranch, options.changed); + const { default: typecheck } = await import('./typecheck'); - await typecheck(); + await typecheck(options, packages); }); interface ServeOptions { diff --git a/packages/modular-scripts/src/test/utils.ts b/packages/modular-scripts/src/test/utils.ts index 782f743f4..f1e7a07a8 100644 --- a/packages/modular-scripts/src/test/utils.ts +++ b/packages/modular-scripts/src/test/utils.ts @@ -4,15 +4,63 @@ import fs, { writeJSONSync } from 'fs-extra'; import path from 'path'; import * as tmp from 'tmp'; import { promisify } from 'util'; +import findUp from 'find-up'; +import { mkdirSync } from 'fs'; -import getModularRoot from '../utils/getModularRoot'; import type { ModularPackageJson } from '@modular-scripts/modular-types'; -import { mkdirSync } from 'fs'; import type { Config } from '@jest/types'; const rimraf = promisify(_rimraf); -const modularRoot = getModularRoot(); +/** + * This is a duplicate copy of `getModularRoot`. + * It exists as a standalone test util so that we can mock the real `getMockRoot` in tests. + * + * A copy of the real implementation (i.e. one that returns the modular root of modular itself) is + * still needed in test code for things like test setup (and the other utilities in here). + */ +export function getRealModularRootInTest(): string { + function isModularRoot(packageJsonPath: string) { + const packageJson = fs.readJSONSync(packageJsonPath, { + encoding: 'utf8', + }) as { modular?: Record }; + return packageJson?.modular?.type === 'root'; + } + + function findModularRoot(): string | undefined { + try { + const modularRoot = findUp.sync( + (directory: string) => { + const packageJsonPath = path.join(directory, 'package.json'); + if ( + findUp.sync.exists(packageJsonPath) && + isModularRoot(packageJsonPath) + ) { + return packageJsonPath; + } + return; + }, + { type: 'file', allowSymlinks: false }, + ); + + return modularRoot + ? path.normalize(path.dirname(modularRoot)) + : undefined; + } catch (err) { + throw new Error(err as string); + } + } + + const modularRoot = findModularRoot(); + + if (modularRoot === undefined) { + throw new Error('Could not find modular root.'); + } + + return modularRoot; +} + +const modularRoot = getRealModularRootInTest(); export async function cleanup(packageNames: Array): Promise { const packagesPath = path.join(modularRoot, 'packages'); @@ -253,3 +301,10 @@ export async function runYarnModular( ...opts, }); } + +export const mockPreflightImplementation = { + __esModule: true, + default: (fn: (...args: unknown[]) => Promise) => { + return fn; + }, +}; diff --git a/packages/modular-scripts/src/typecheck.ts b/packages/modular-scripts/src/typecheck.ts index 4d4a49a75..6f861f5ab 100644 --- a/packages/modular-scripts/src/typecheck.ts +++ b/packages/modular-scripts/src/typecheck.ts @@ -5,35 +5,107 @@ import getPackageMetadata from './utils/getPackageMetadata'; import * as logger from './utils/logger'; import getModularRoot from './utils/getModularRoot'; import actionPreflightCheck from './utils/actionPreflightCheck'; +import { getAllWorkspaces } from './utils/getAllWorkspaces'; +import { selectWorkspaces } from './utils/selectWorkspaces'; -async function typecheck(): Promise { - const { typescriptConfig } = await getPackageMetadata(); +import type { JSONSchemaForTheTypeScriptCompilerSConfigurationFile as TSConfig } from '@schemastore/tsconfig'; + +type CompilerOptions = TSConfig['compilerOptions']; - const { _compilerOptions, ...rest } = typescriptConfig; +export interface TypecheckOptions { + ancestors: boolean; + descendants: boolean; + changed: boolean; + compareBranch: string; +} - const tsConfig: typeof typescriptConfig = { +const COMPILER_OPTIONS_ALLOW_LIST: string[] = ['jsx']; +const DEFAULT_COMPILER_OPTIONS: CompilerOptions = { + noEmit: true, +}; +const DEFAULT_EXCLUDES: string[] = [ + 'node_modules', + 'bower_components', + 'jspm_packages', + 'tmp', + '**/dist-types', + '**/dist-cjs', + '**/dist-es', + 'dist', + '**/__fixtures__', +]; + +function restrictUserTsconfig( + userTsConfig: TSConfig, + replaceInclude: string[] | undefined, +): TSConfig { + const { compilerOptions, ...rest } = userTsConfig; + + // The exclude list here is different to that in getPackageInfo() because we want + // to test more files during a typecheck compared to a build (test/spec files). During a build, + // there might be various files that we don't want to end up in the output. + const tsConfig: TSConfig = { ...rest, - exclude: [ - 'node_modules', - 'bower_components', - 'jspm_packages', - 'tmp', - '**/dist-types', - '**/dist-cjs', - '**/dist-es', - 'dist', - '**/__fixtures__', - ], - compilerOptions: { - noEmit: true, - }, + exclude: [...DEFAULT_EXCLUDES], + }; + + const combinedCompilerOptions: CompilerOptions = { + ...DEFAULT_COMPILER_OPTIONS, }; - // If compilerOptions.jsx is defined use it otherwise, depend on the extended config by not setting it to undefined. - if (tsConfig.compilerOptions && rest.compilerOptions?.jsx) { - tsConfig.compilerOptions.jsx = rest.compilerOptions.jsx; + // Where the user has defined allowlist-supported compiler options, apply them + for (const item of COMPILER_OPTIONS_ALLOW_LIST) { + if (compilerOptions && compilerOptions[item]) { + // eslint-disable-next-line @typescript-eslint/no-unsafe-assignment + combinedCompilerOptions[item] = compilerOptions[item]; + } + } + + tsConfig.compilerOptions = combinedCompilerOptions; + + // If the user provided selective options, use a computed `include` config instead of what is derived from tsconfig.json + if (replaceInclude) { + tsConfig.include = replaceInclude; + } + + return tsConfig; +} + +async function typecheck( + options: TypecheckOptions, + packages: string[], +): Promise { + const { ancestors, descendants, changed, compareBranch } = options; + const isSelective = changed || ancestors || descendants || packages.length; + const modularRoot = getModularRoot(); + const [allWorkspacePackages] = await getAllWorkspaces(modularRoot); + const targets = isSelective ? packages : [...allWorkspacePackages.keys()]; + let replaceInclude: undefined | string[]; + + if (isSelective) { + const selectedTargets = await selectWorkspaces({ + targets, + changed, + compareBranch, + descendants, + ancestors, + }); + + const targetLocations: string[] = []; + for (const [pkgName, pkg] of allWorkspacePackages) { + if (selectedTargets.includes(pkgName)) { + targetLocations.push(pkg.location); + } + } + + if (targetLocations.length) { + replaceInclude = targetLocations; + } } + const { typescriptConfig } = await getPackageMetadata(); + const tsConfig = restrictUserTsconfig(typescriptConfig, replaceInclude); + const diagnosticHost = { getCurrentDirectory: (): string => getModularRoot(), getNewLine: (): string => ts.sys.newLine, @@ -45,7 +117,7 @@ async function typecheck(): Promise { const configParseResult = ts.parseJsonConfigFileContent( tsConfig, ts.sys, - getModularRoot(), + modularRoot, ); if (configParseResult.errors.length > 0) { diff --git a/packages/modular-scripts/src/utils/getPackageMetadata.ts b/packages/modular-scripts/src/utils/getPackageMetadata.ts index afc139c04..57971b32c 100644 --- a/packages/modular-scripts/src/utils/getPackageMetadata.ts +++ b/packages/modular-scripts/src/utils/getPackageMetadata.ts @@ -56,9 +56,10 @@ async function getPackageMetadata() { // validate tsconfig // Extract configuration from config file and parse JSON, // after removing comments. Just a fancier JSON.parse + const userTsConfigPath = path.join(modularRoot, typescriptConfigFilename); const result = ts.parseConfigFileTextToJson( - path.join(modularRoot, typescriptConfigFilename), - fse.readFileSync(typescriptConfigFilename, 'utf8').toString(), + userTsConfigPath, + fse.readFileSync(userTsConfigPath, 'utf8').toString(), ); const configObject = result.config as TSConfig; diff --git a/packages/modular-scripts/src/utils/validateOptions.ts b/packages/modular-scripts/src/utils/validateOptions.ts new file mode 100644 index 000000000..24521db14 --- /dev/null +++ b/packages/modular-scripts/src/utils/validateOptions.ts @@ -0,0 +1,16 @@ +/** + * Validates comparison options in combination. + * + * Throws and exits when invalid combinations are detected. + */ +export function validateCompareOptions( + compareBranch: string | undefined, + changed: boolean, +): void { + if (compareBranch && !changed) { + process.stderr.write( + "Option --compareBranch doesn't make sense without option --changed\n", + ); + process.exit(1); + } +}