Skip to content

Commit

Permalink
Parsing Speedup by Adding Source Files via tsconfig (#357)
Browse files Browse the repository at this point in the history
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](#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>
  • Loading branch information
oshi97 and github-actions[bot] authored Aug 30, 2023
1 parent 2244f4d commit d7b3262
Show file tree
Hide file tree
Showing 20 changed files with 65 additions and 43 deletions.
19 changes: 12 additions & 7 deletions .github/workflows/run-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion e2e-tests/playwright.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
/**
Expand Down
File renamed without changes.
1 change: 1 addition & 0 deletions e2e-tests/tests/infra/setupAcceptance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
7 changes: 7 additions & 0 deletions e2e-tests/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"include": ["src", "tests", "global.d.ts"],
"compilerOptions": {
"jsx": "react-jsx",
"esModuleInterop": true
}
}
3 changes: 2 additions & 1 deletion packages/studio-plugin/src/ParsingOrchestrator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
4 changes: 3 additions & 1 deletion packages/studio-plugin/src/createStudioPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
9 changes: 4 additions & 5 deletions packages/studio-plugin/tests/FileSystemManager.test.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -13,6 +11,7 @@ import {
FileMetadataKind,
PageState,
} from "../src/types";
import { createTestProject } from "./__utils__/createTestSourceFile";

const bannerComponentState: ComponentState = {
kind: ComponentStateKind.Standard,
Expand All @@ -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");
Expand Down Expand Up @@ -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
Expand Down
7 changes: 3 additions & 4 deletions packages/studio-plugin/tests/ParsingOrchestrator.test.ts
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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");

Expand Down Expand Up @@ -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,
{
Expand Down
12 changes: 10 additions & 2 deletions packages/studio-plugin/tests/__utils__/createTestSourceFile.ts
Original file line number Diff line number Diff line change
@@ -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",
},
});
}
2 changes: 1 addition & 1 deletion packages/studio-plugin/tests/parsers/GetPathParser.test.ts
Original file line number Diff line number Diff line change
@@ -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", () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -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();

Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import ComponentFile from "../../src/sourcefiles/ComponentFile";
import { createTsMorphProject } from "../../src/ParsingOrchestrator";
import { getComponentPath } from "../__utils__/getFixturePath";
import {
ComponentMetadata,
Expand All @@ -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");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand Down Expand Up @@ -40,7 +41,7 @@ const mockGetFileMetadata: GetFileMetadata = (filepath: string) => {
};

describe("getModuleMetadata", () => {
const project = createTsMorphProject();
const project = createTestProject();
beforeEach(() => {
mockUUID();
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand All @@ -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"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down Expand Up @@ -43,7 +43,7 @@ function createPageFile(pageName: string, isPagesJSRepo = false): PageFile {
return new PageFile(
getPagePath(pageName),
mockGetFileMetadata,
createTsMorphProject(),
createTestProject(),
isPagesJSRepo
);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand All @@ -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", () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -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", () => {
Expand Down
7 changes: 3 additions & 4 deletions packages/studio-plugin/tests/writers/FileSystemWriter.test.ts
Original file line number Diff line number Diff line change
@@ -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";
Expand All @@ -12,6 +10,7 @@ import {
FileMetadataKind,
ModuleMetadata,
} from "../../src/types";
import { createTestProject } from "../__utils__/createTestSourceFile";

jest.mock("fs", () => {
const actualFs = jest.requireActual("fs");
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down Expand Up @@ -61,7 +61,7 @@ describe("updateFile", () => {
let tsMorphProject: Project;
beforeEach(() => {
jest.spyOn(fs, "writeFileSync").mockImplementation();
tsMorphProject = createTsMorphProject();
tsMorphProject = createTestProject();
});

describe("React component return statement", () => {
Expand Down

0 comments on commit d7b3262

Please sign in to comment.