From d7b32625daf06d603ee8b83ccb50bd2074b79a4b Mon Sep 17 00:00:00 2001 From: Oliver Shi Date: Wed, 30 Aug 2023 16:00:06 -0400 Subject: [PATCH] Parsing Speedup by Adding Source Files via tsconfig (#357) This PR updates our parsing classes to avoid dynamically add source files to the project during initial parsing. Instead, we specify a path to the user's tsconfig, so that ts-morph can add them all at once on startup. Originally, I wanted to never dynamically add source files. However, this is still necessary for when we add new files, at least not without reworking at least a good portion of how we write to file. This speeds up initial parsing because every time a source file is added to the project (for instance via `addSourceFileAtPath` like we were doing), ts-morph will recreate the entire typescript compiler Project instance, which is a slow operation. I update the unit test workflow to split up the studio/studio-plugin tests, and also turn off coverage collection to speed up tests. We can turn it back on when we actually do something with it. We can also drop down the playwright test timeout now. The performance improvements here help, and also when we [updated the pages dev startup to use the api](https://github.com/yext/studio/pull/348) instead of a spawnSync. J=SLAP-2909 TEST=manual my local test-site has speed up to ~6 second start time, down from ~18 seconds --------- Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com> --- .github/workflows/run-tests.yml | 19 ++++++++++++------- e2e-tests/playwright.config.ts | 2 +- ...omponent.spec.ts => add-component.spec.ts} | 0 e2e-tests/tests/infra/setupAcceptance.ts | 1 + e2e-tests/tsconfig.json | 7 +++++++ .../studio-plugin/src/ParsingOrchestrator.ts | 3 ++- .../studio-plugin/src/createStudioPlugin.ts | 4 +++- .../tests/FileSystemManager.test.ts | 9 ++++----- .../tests/ParsingOrchestrator.test.ts | 7 +++---- .../tests/__utils__/createTestSourceFile.ts | 12 ++++++++++-- .../tests/parsers/GetPathParser.test.ts | 2 +- .../parsers/TypeNodeParsingHelper.test.ts | 2 +- .../tests/sourcefiles/ComponentFile.test.ts | 4 ++-- .../ModuleFile.getModuleMetadata.test.ts | 7 ++++--- .../ModuleFile.updateModuleFile.test.ts | 6 +++--- .../sourcefiles/PageFile.getPageState.test.ts | 4 ++-- .../PageFile.updatePageFile.test.ts | 4 ++-- .../sourcefiles/SiteSettingsFile.test.ts | 4 ++-- .../tests/writers/FileSystemWriter.test.ts | 7 +++---- .../writers/ReactComponentFileWriter.test.ts | 4 ++-- 20 files changed, 65 insertions(+), 43 deletions(-) rename e2e-tests/tests/hmr/{add.component.spec.ts => add-component.spec.ts} (100%) create mode 100644 e2e-tests/tsconfig.json diff --git a/.github/workflows/run-tests.yml b/.github/workflows/run-tests.yml index b5abb7613..ec10faa1c 100644 --- a/.github/workflows/run-tests.yml +++ b/.github/workflows/run-tests.yml @@ -2,21 +2,26 @@ name: Run Tests on: workflow_call jobs: - unit_tests: + studio: strategy: matrix: os: [windows-latest, ubuntu-latest] - node-version: [18.x] runs-on: ${{ matrix.os }} steps: - uses: actions/checkout@v3 - uses: ./.github/setup-ci - with: - node-version: ${{ matrix.node-version }} - run: npm run build -w=packages/studio-plugin - - run: npm test -w=packages/studio-plugin - - run: npm test -w=packages/studio - if: success() || failure() + - run: npm test -w=packages/studio -- --coverage=false + + studio_plugin: + strategy: + matrix: + os: [windows-latest, ubuntu-latest] + runs-on: ${{ matrix.os }} + steps: + - uses: actions/checkout@v3 + - uses: ./.github/setup-ci + - run: npm test -w=packages/studio-plugin -- --coverage=false test_build: strategy: diff --git a/e2e-tests/playwright.config.ts b/e2e-tests/playwright.config.ts index 0276e0e64..94885709e 100644 --- a/e2e-tests/playwright.config.ts +++ b/e2e-tests/playwright.config.ts @@ -23,7 +23,7 @@ expect.extend({ const config: PlaywrightTestConfig = { testDir: "./tests", /* Maximum time one test can run for. */ - timeout: 150 * 1000, + timeout: 75 * 1000, snapshotPathTemplate: "__screenshots__/{platform}/{testFilePath}/{arg}{ext}", expect: { /** diff --git a/e2e-tests/tests/hmr/add.component.spec.ts b/e2e-tests/tests/hmr/add-component.spec.ts similarity index 100% rename from e2e-tests/tests/hmr/add.component.spec.ts rename to e2e-tests/tests/hmr/add-component.spec.ts diff --git a/e2e-tests/tests/infra/setupAcceptance.ts b/e2e-tests/tests/infra/setupAcceptance.ts index 2866f5599..ecfc4d379 100644 --- a/e2e-tests/tests/infra/setupAcceptance.ts +++ b/e2e-tests/tests/infra/setupAcceptance.ts @@ -99,6 +99,7 @@ function createTempWorkingDir(testInfo: TestInfo, tailwindConfigPath?: string) { copy("localData"); copy("sites-config"); copy("studio.config.js"); + copy("tsconfig.json"); if (tailwindConfigPath) { fsExtra.copySync( diff --git a/e2e-tests/tsconfig.json b/e2e-tests/tsconfig.json new file mode 100644 index 000000000..f7b2c98b6 --- /dev/null +++ b/e2e-tests/tsconfig.json @@ -0,0 +1,7 @@ +{ + "include": ["src", "tests", "global.d.ts"], + "compilerOptions": { + "jsx": "react-jsx", + "esModuleInterop": true + } +} diff --git a/packages/studio-plugin/src/ParsingOrchestrator.ts b/packages/studio-plugin/src/ParsingOrchestrator.ts index 123d812bb..132cf17ad 100644 --- a/packages/studio-plugin/src/ParsingOrchestrator.ts +++ b/packages/studio-plugin/src/ParsingOrchestrator.ts @@ -21,8 +21,9 @@ import typescript from "typescript"; import { v4 } from "uuid"; import { ParsingError } from "./errors/ParsingError"; -export function createTsMorphProject() { +export function createTsMorphProject(tsConfigFilePath: string) { return new Project({ + tsConfigFilePath, compilerOptions: { jsx: typescript.JsxEmit.ReactJSX, }, diff --git a/packages/studio-plugin/src/createStudioPlugin.ts b/packages/studio-plugin/src/createStudioPlugin.ts index ac063ea26..ad78c6d97 100644 --- a/packages/studio-plugin/src/createStudioPlugin.ts +++ b/packages/studio-plugin/src/createStudioPlugin.ts @@ -48,7 +48,9 @@ export default async function createStudioPlugin( await gitWrapper.setup(); /** The ts-morph Project instance for the entire app. */ - const tsMorphProject = createTsMorphProject(); + const tsMorphProject = createTsMorphProject( + upath.join(pathToUserProjectRoot, "tsconfig.json") + ); const localDataMappingManager = studioConfig.isPagesJSRepo ? new LocalDataMappingManager(studioConfig.paths.localData) : undefined; diff --git a/packages/studio-plugin/tests/FileSystemManager.test.ts b/packages/studio-plugin/tests/FileSystemManager.test.ts index c6565b026..95c40a161 100644 --- a/packages/studio-plugin/tests/FileSystemManager.test.ts +++ b/packages/studio-plugin/tests/FileSystemManager.test.ts @@ -1,8 +1,6 @@ import { Project } from "ts-morph"; import FileSystemManager from "../src/FileSystemManager"; -import ParsingOrchestrator, { - createTsMorphProject, -} from "../src/ParsingOrchestrator"; +import ParsingOrchestrator from "../src/ParsingOrchestrator"; import getUserPaths from "../src/parsers/getUserPaths"; import upath from "upath"; import fs from "fs"; @@ -13,6 +11,7 @@ import { FileMetadataKind, PageState, } from "../src/types"; +import { createTestProject } from "./__utils__/createTestSourceFile"; const bannerComponentState: ComponentState = { kind: ComponentStateKind.Standard, @@ -32,7 +31,7 @@ const projectRoot = upath.resolve( __dirname, "./__fixtures__/FileSystemManager" ); -const tsMorphProject: Project = createTsMorphProject(); +const tsMorphProject: Project = createTestProject(); const paths = getUserPaths(projectRoot); paths.pages = upath.join(projectRoot, "pages"); paths.modules = upath.join(projectRoot, "modules"); @@ -87,7 +86,7 @@ describe("updatePageFile", () => { ); }); - it("creates a new page file and add a page component based on new state", () => { + it("creates a new page file and adds a page component based on new state", () => { jest.spyOn(fs, "existsSync").mockImplementation(() => false); const fsMkdirSyncSpy = jest.spyOn(fs, "mkdirSync").mockImplementation(); const fsOpenSyncSpy = jest diff --git a/packages/studio-plugin/tests/ParsingOrchestrator.test.ts b/packages/studio-plugin/tests/ParsingOrchestrator.test.ts index 4b66ffaab..c046377c5 100644 --- a/packages/studio-plugin/tests/ParsingOrchestrator.test.ts +++ b/packages/studio-plugin/tests/ParsingOrchestrator.test.ts @@ -1,6 +1,4 @@ -import ParsingOrchestrator, { - createTsMorphProject, -} from "../src/ParsingOrchestrator"; +import ParsingOrchestrator from "../src/ParsingOrchestrator"; import getUserPaths from "../src/parsers/getUserPaths"; import upath from "upath"; import { @@ -15,6 +13,7 @@ import { Project } from "ts-morph"; import fs from "fs"; import prettyPrintError from "../src/errors/prettyPrintError"; import { assertIsOk } from "./__utils__/asserts"; +import { createTestProject } from "./__utils__/createTestSourceFile"; jest.mock("../src/errors/prettyPrintError"); @@ -233,7 +232,7 @@ function createParsingOrchestrator(opts?: { isPagesJS?: boolean; }) { const { getLocalDataMapping, paths, isPagesJS } = opts ?? {}; - const tsMorphProject: Project = createTsMorphProject(); + const tsMorphProject: Project = createTestProject(); const orchestrator = new ParsingOrchestrator( tsMorphProject, { diff --git a/packages/studio-plugin/tests/__utils__/createTestSourceFile.ts b/packages/studio-plugin/tests/__utils__/createTestSourceFile.ts index 59e6fbc8d..59a9b11b1 100644 --- a/packages/studio-plugin/tests/__utils__/createTestSourceFile.ts +++ b/packages/studio-plugin/tests/__utils__/createTestSourceFile.ts @@ -1,13 +1,21 @@ -import { createTsMorphProject } from "../../src/ParsingOrchestrator"; +import { Project } from "ts-morph"; export default function createTestSourceFile( code: string, filepath = "test.tsx" ) { - const p = createTsMorphProject(); + const p = createTestProject(); p.createSourceFile(filepath, code); return { sourceFile: p.getSourceFileOrThrow(filepath), project: p, }; } + +export function createTestProject() { + return new Project({ + compilerOptions: { + react: "react-jsx", + }, + }); +} diff --git a/packages/studio-plugin/tests/parsers/GetPathParser.test.ts b/packages/studio-plugin/tests/parsers/GetPathParser.test.ts index cbb072d28..8a18e1348 100644 --- a/packages/studio-plugin/tests/parsers/GetPathParser.test.ts +++ b/packages/studio-plugin/tests/parsers/GetPathParser.test.ts @@ -1,6 +1,6 @@ -import { PropValueKind } from "../../lib"; import GetPathParser from "../../src/parsers/GetPathParser"; import StudioSourceFileParser from "../../src/parsers/StudioSourceFileParser"; +import { PropValueKind } from "../../src/types"; import createTestSourceFile from "../__utils__/createTestSourceFile"; describe("parseGetPathValue", () => { diff --git a/packages/studio-plugin/tests/parsers/TypeNodeParsingHelper.test.ts b/packages/studio-plugin/tests/parsers/TypeNodeParsingHelper.test.ts index 613a019d7..af57cb136 100644 --- a/packages/studio-plugin/tests/parsers/TypeNodeParsingHelper.test.ts +++ b/packages/studio-plugin/tests/parsers/TypeNodeParsingHelper.test.ts @@ -1,9 +1,9 @@ import { SyntaxKind } from "ts-morph"; -import { PropValueType } from "../../lib"; import TypeNodeParsingHelper, { ParsedTypeKind, } from "../../src/parsers/helpers/TypeNodeParsingHelper"; import createTestSourceFile from "../__utils__/createTestSourceFile"; +import { PropValueType } from "../../src/types"; const externalShapeParser = jest.fn(); diff --git a/packages/studio-plugin/tests/sourcefiles/ComponentFile.test.ts b/packages/studio-plugin/tests/sourcefiles/ComponentFile.test.ts index a1cc78820..1577c60a2 100644 --- a/packages/studio-plugin/tests/sourcefiles/ComponentFile.test.ts +++ b/packages/studio-plugin/tests/sourcefiles/ComponentFile.test.ts @@ -1,5 +1,4 @@ import ComponentFile from "../../src/sourcefiles/ComponentFile"; -import { createTsMorphProject } from "../../src/ParsingOrchestrator"; import { getComponentPath } from "../__utils__/getFixturePath"; import { ComponentMetadata, @@ -9,12 +8,13 @@ import { } from "../../src/types"; import { assertIsOk } from "../__utils__/asserts"; import upath from "upath"; +import { createTestProject } from "../__utils__/createTestSourceFile"; describe("getComponentMetadata", () => { beforeEach(() => { jest.spyOn(console, "error").mockImplementation(); }); - const project = createTsMorphProject(); + const project = createTestProject(); it("can parse a simple Banner component", () => { const pathToComponent = getComponentPath("SimpleBanner"); diff --git a/packages/studio-plugin/tests/sourcefiles/ModuleFile.getModuleMetadata.test.ts b/packages/studio-plugin/tests/sourcefiles/ModuleFile.getModuleMetadata.test.ts index fc39be9e5..7c91d12f7 100644 --- a/packages/studio-plugin/tests/sourcefiles/ModuleFile.getModuleMetadata.test.ts +++ b/packages/studio-plugin/tests/sourcefiles/ModuleFile.getModuleMetadata.test.ts @@ -9,10 +9,11 @@ import { } from "../../src/types"; import { mockUUID } from "../__utils__/spies"; import { GetFileMetadata } from "../../src/parsers/ComponentTreeParser"; -import { createTsMorphProject } from "../../src/ParsingOrchestrator"; import ModuleFile from "../../src/sourcefiles/ModuleFile"; import { assertIsOk } from "../__utils__/asserts"; -import createTestSourceFile from "../__utils__/createTestSourceFile"; +import createTestSourceFile, { + createTestProject, +} from "../__utils__/createTestSourceFile"; import upath from "upath"; jest.mock("uuid"); @@ -40,7 +41,7 @@ const mockGetFileMetadata: GetFileMetadata = (filepath: string) => { }; describe("getModuleMetadata", () => { - const project = createTsMorphProject(); + const project = createTestProject(); beforeEach(() => { mockUUID(); }); diff --git a/packages/studio-plugin/tests/sourcefiles/ModuleFile.updateModuleFile.test.ts b/packages/studio-plugin/tests/sourcefiles/ModuleFile.updateModuleFile.test.ts index eba2cb0f1..e1f055006 100644 --- a/packages/studio-plugin/tests/sourcefiles/ModuleFile.updateModuleFile.test.ts +++ b/packages/studio-plugin/tests/sourcefiles/ModuleFile.updateModuleFile.test.ts @@ -9,13 +9,13 @@ import { PropValues, PropValueType, PropShape, + TypelessPropVal, } from "../../src/types"; import { getComponentPath, getModulePath } from "../__utils__/getFixturePath"; import { addFilesToProject } from "../__utils__/addFilesToProject"; import { complexBannerComponent } from "../__fixtures__/componentStates"; import { throwIfCalled } from "../__utils__/throwIfCalled"; -import { createTsMorphProject } from "../../src/ParsingOrchestrator"; -import { TypelessPropVal } from "../../lib"; +import { createTestProject } from "../__utils__/createTestSourceFile"; jest.mock("uuid"); @@ -24,7 +24,7 @@ describe("updateModuleFile", () => { let moduleFile: ModuleFile; beforeEach(() => { jest.spyOn(fs, "writeFileSync").mockImplementation(); - tsMorphProject = createTsMorphProject(); + tsMorphProject = createTestProject(); addFilesToProject(tsMorphProject, [getComponentPath("ComplexBanner")]); moduleFile = new ModuleFile( getModulePath("updateModuleFile/EmptyModule"), diff --git a/packages/studio-plugin/tests/sourcefiles/PageFile.getPageState.test.ts b/packages/studio-plugin/tests/sourcefiles/PageFile.getPageState.test.ts index 2a7c77bef..fffcdec16 100644 --- a/packages/studio-plugin/tests/sourcefiles/PageFile.getPageState.test.ts +++ b/packages/studio-plugin/tests/sourcefiles/PageFile.getPageState.test.ts @@ -8,9 +8,9 @@ import { fragmentComponent, nestedBannerComponentTree, } from "../__fixtures__/componentStates"; -import { createTsMorphProject } from "../../src/ParsingOrchestrator"; import { mockUUID } from "../__utils__/spies"; import { assertIsOk } from "../__utils__/asserts"; +import { createTestProject } from "../__utils__/createTestSourceFile"; jest.mock("uuid"); @@ -43,7 +43,7 @@ function createPageFile(pageName: string, isPagesJSRepo = false): PageFile { return new PageFile( getPagePath(pageName), mockGetFileMetadata, - createTsMorphProject(), + createTestProject(), isPagesJSRepo ); } diff --git a/packages/studio-plugin/tests/sourcefiles/PageFile.updatePageFile.test.ts b/packages/studio-plugin/tests/sourcefiles/PageFile.updatePageFile.test.ts index d53c8dc9e..ac4c64dd1 100644 --- a/packages/studio-plugin/tests/sourcefiles/PageFile.updatePageFile.test.ts +++ b/packages/studio-plugin/tests/sourcefiles/PageFile.updatePageFile.test.ts @@ -8,9 +8,9 @@ import { Project } from "ts-morph"; import { streamConfigMultipleFieldsComponentTree } from "../__fixtures__/componentStates"; import { addFilesToProject } from "../__utils__/addFilesToProject"; import { throwIfCalled } from "../__utils__/throwIfCalled"; -import { createTsMorphProject } from "../../src/ParsingOrchestrator"; import { PageState } from "../../src/types/PageState"; import upath from "upath"; +import { createTestProject } from "../__utils__/createTestSourceFile"; jest.mock("uuid"); @@ -32,7 +32,7 @@ describe("updatePageFile", () => { let tsMorphProject: Project; beforeEach(() => { jest.spyOn(fs, "writeFileSync").mockImplementation(); - tsMorphProject = createTsMorphProject(); + tsMorphProject = createTestProject(); }); it("updates page component based on PageState's component tree", () => { diff --git a/packages/studio-plugin/tests/sourcefiles/SiteSettingsFile.test.ts b/packages/studio-plugin/tests/sourcefiles/SiteSettingsFile.test.ts index a170b51a4..4a2f4f24a 100644 --- a/packages/studio-plugin/tests/sourcefiles/SiteSettingsFile.test.ts +++ b/packages/studio-plugin/tests/sourcefiles/SiteSettingsFile.test.ts @@ -1,14 +1,14 @@ import { PropValueKind, PropValueType, SiteSettings } from "../../src/types"; import SiteSettingsFile from "../../src/sourcefiles/SiteSettingsFile"; -import { createTsMorphProject } from "../../src/ParsingOrchestrator"; import { getSiteSettingsPath } from "../__utils__/getFixturePath"; import fs from "fs"; import { Project } from "ts-morph"; +import { createTestProject } from "../__utils__/createTestSourceFile"; let tsMorphProject: Project; beforeEach(() => { jest.spyOn(fs, "writeFileSync").mockImplementation(); - tsMorphProject = createTsMorphProject(); + tsMorphProject = createTestProject(); }); describe("getSiteSettings", () => { diff --git a/packages/studio-plugin/tests/writers/FileSystemWriter.test.ts b/packages/studio-plugin/tests/writers/FileSystemWriter.test.ts index 3ee832d57..ebd35e580 100644 --- a/packages/studio-plugin/tests/writers/FileSystemWriter.test.ts +++ b/packages/studio-plugin/tests/writers/FileSystemWriter.test.ts @@ -1,7 +1,5 @@ import { Project } from "ts-morph"; -import ParsingOrchestrator, { - createTsMorphProject, -} from "../../src/ParsingOrchestrator"; +import ParsingOrchestrator from "../../src/ParsingOrchestrator"; import getUserPaths from "../../src/parsers/getUserPaths"; import upath from "upath"; import fs from "fs"; @@ -12,6 +10,7 @@ import { FileMetadataKind, ModuleMetadata, } from "../../src/types"; +import { createTestProject } from "../__utils__/createTestSourceFile"; jest.mock("fs", () => { const actualFs = jest.requireActual("fs"); @@ -53,7 +52,7 @@ const moduleMetadata: ModuleMetadata = { }; describe("syncFileMetadata", () => { - const tsMorphProject: Project = createTsMorphProject(); + const tsMorphProject: Project = createTestProject(); const orchestrator = new ParsingOrchestrator(tsMorphProject, { paths, openBrowser: true, diff --git a/packages/studio-plugin/tests/writers/ReactComponentFileWriter.test.ts b/packages/studio-plugin/tests/writers/ReactComponentFileWriter.test.ts index 066eba637..3d486aa07 100644 --- a/packages/studio-plugin/tests/writers/ReactComponentFileWriter.test.ts +++ b/packages/studio-plugin/tests/writers/ReactComponentFileWriter.test.ts @@ -25,7 +25,7 @@ import { } from "../../src/types"; import StudioSourceFileWriter from "../../src/writers/StudioSourceFileWriter"; import StudioSourceFileParser from "../../src/parsers/StudioSourceFileParser"; -import { createTsMorphProject } from "../../src/ParsingOrchestrator"; +import { createTestProject } from "../__utils__/createTestSourceFile"; const propShapeMultiFields: PropShape = { complexBannerText: { @@ -61,7 +61,7 @@ describe("updateFile", () => { let tsMorphProject: Project; beforeEach(() => { jest.spyOn(fs, "writeFileSync").mockImplementation(); - tsMorphProject = createTsMorphProject(); + tsMorphProject = createTestProject(); }); describe("React component return statement", () => {