ci(type-checking): run type-checking-frontend hook sequentially #32323

alveifbklsiu259 · 2025-02-20T05:45:13Z

ci(type-checking): run type-checking-frontend hook sequentially

SUMMARY

Run type-checking-frontend hook sequentially by setting require_serial: true.

By default, pre-commit runs each hook in parallel with different set of matched files:

pre-commit-config.yaml

- id: type-checking-frontend
  name: Type-Checking (Frontend)
  entry: ./scripts/check-type.js
  language: node
  files: ^superset-frontend\/.*\.(js|jsx|ts|tsx)$
  exclude: ^superset-frontend/cypress-base\/

check-type.js

void (async () => {
  const args = process.argv.slice(2);
  console.log("the script is run with the args:", args);
  exit(1);
}

git status
On branch ci/type-checking
Changes to be committed:
  (use "git restore --staged <file>..." to unstage)
        modified:   .pre-commit-config.yaml
        modified:   scripts/check-type.js
        modified:   superset-frontend/package-lock.json
        modified:   superset-frontend/src/dashboard/actions/dashboardInfo.ts
        modified:   superset-frontend/src/dashboard/actions/datasources.ts
        modified:   superset-frontend/src/dashboard/actions/nativeFilters.ts
        modified:   superset-frontend/src/dashboard/actions/sliceEntities.ts
        modified:   superset-frontend/src/dashboard/components/AddSliceCard/AddSliceCard.test.tsx
        modified:   superset-frontend/src/dashboard/components/AddSliceCard/AddSliceCard.tsx
        modified:   superset-frontend/src/dashboard/components/AddSliceCard/index.ts
        modified:   superset-frontend/src/dashboard/components/AnchorLink/AnchorLink.stories.tsx
        modified:   superset-frontend/src/dashboard/components/AnchorLink/AnchorLink.test.tsx
        modified:   superset-frontend/src/dashboard/components/AnchorLink/index.tsx
        modified:   superset-frontend/src/dashboard/components/BuilderComponentPane/BuilderComponentPane.test.tsx
        modified:   superset-frontend/src/dashboard/components/BuilderComponentPane/index.tsx
        modified:   superset-frontend/src/dashboard/components/CssEditor/index.tsx
        modified:   superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx
        modified:   superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx
        modified:   superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx
        modified:   superset-frontend/src/dashboard/components/DashboardBuilder/DashboardWrapper.test.tsx
        modified:   superset-frontend/src/dashboard/components/DashboardBuilder/DashboardWrapper.tsx
        modified:   superset-frontend/src/dashboard/components/DashboardBuilder/state.ts
        modified:   superset-frontend/src/explore/actions/datasourcesActions.ts
        modified:   superset-frontend/src/explore/components/controls/ViewQuery.tsx
        new file:   superset-frontend/src/types/jest-styled-components.d.ts
        modified:   superset-frontend/tsconfig.json

The script is run 6 times. Which means that tsc will be run 6 times.

pre-commit run type-checking-frontend 
run type-checking-frontend 
Type-Checking (Frontend).................................................Failed
- hook id: type-checking-frontend
- exit code: 1

the script is run with the args: [
  'superset-frontend/src/dashboard/components/AddSliceCard/AddSliceCard.test.tsx',
  'superset-frontend/src/dashboard/components/AddSliceCard/AddSliceCard.tsx',
  'superset-frontend/src/dashboard/components/AnchorLink/AnchorLink.stories.tsx',
  'superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx'
]
the script is run with the args: [
  'superset-frontend/src/dashboard/components/CssEditor/index.tsx',
  'superset-frontend/src/dashboard/actions/dashboardInfo.ts',
  'superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx',
  'superset-frontend/src/dashboard/components/BuilderComponentPane/index.tsx'
]
the script is run with the args: [
  'superset-frontend/src/dashboard/components/AnchorLink/index.tsx',
  'superset-frontend/src/dashboard/actions/datasources.ts',
  'superset-frontend/src/types/jest-styled-components.d.ts',
  'superset-frontend/src/dashboard/components/DashboardBuilder/DashboardWrapper.test.tsx'
]
the script is run with the args: [
  'superset-frontend/src/dashboard/actions/nativeFilters.ts',
  'superset-frontend/src/dashboard/components/AnchorLink/AnchorLink.test.tsx',
  'superset-frontend/src/dashboard/actions/sliceEntities.ts',
  'superset-frontend/src/explore/components/controls/ViewQuery.tsx'
]
the script is run with the args: [
  'superset-frontend/src/dashboard/components/DashboardBuilder/DashboardWrapper.tsx',
  'superset-frontend/src/dashboard/components/BuilderComponentPane/BuilderComponentPane.test.tsx',
  'superset-frontend/src/dashboard/components/AddSliceCard/index.ts',
  'superset-frontend/src/explore/actions/datasourcesActions.ts'
]
the script is run with the args: [
  'superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx',
  'superset-frontend/src/dashboard/components/DashboardBuilder/state.ts'
]

Potential issues:

Each invocation of tsc incurs a startup overhead. Running tsc with different set of files multiple times in parallel may actually be slower than running it once with all the files.
Running tsc multiple times in parallel may show the same error multiple times in the console.

pre-commit-config.yaml

- id: type-checking-frontend
  name: Type-Checking (Frontend)
  entry: ./scripts/check-type.js
  args: [package=superset-frontend, excludeDeclarationDir=cypress-base]
  language: node
  files: ^superset-frontend\/.*\.(js|jsx|ts|tsx)$
  exclude: ^superset-frontend/cypress-base\/

git status
On branch ci/type-checking
Changes to be committed:
  (use "git restore --staged <file>..." to unstage)
        modified:   superset-frontend/package-lock.json
        modified:   superset-frontend/src/dashboard/actions/dashboardInfo.ts
        modified:   superset-frontend/src/dashboard/actions/datasources.ts
        modified:   superset-frontend/src/dashboard/actions/nativeFilters.ts
        modified:   superset-frontend/src/dashboard/actions/sliceEntities.ts
        modified:   superset-frontend/src/dashboard/components/AddSliceCard/AddSliceCard.test.tsx
        modified:   superset-frontend/src/dashboard/components/AddSliceCard/AddSliceCard.tsx
        modified:   superset-frontend/src/dashboard/components/AddSliceCard/index.ts
        modified:   superset-frontend/src/dashboard/components/AnchorLink/AnchorLink.stories.tsx
        modified:   superset-frontend/src/dashboard/components/AnchorLink/AnchorLink.test.tsx
        modified:   superset-frontend/src/dashboard/components/AnchorLink/index.tsx
        modified:   superset-frontend/src/dashboard/components/BuilderComponentPane/BuilderComponentPane.test.tsx
        modified:   superset-frontend/src/dashboard/components/BuilderComponentPane/index.tsx
        modified:   superset-frontend/src/dashboard/components/CssEditor/index.tsx
        modified:   superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.test.tsx
        modified:   superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx
        modified:   superset-frontend/src/dashboard/components/DashboardBuilder/DashboardContainer.tsx
        modified:   superset-frontend/src/dashboard/components/DashboardBuilder/DashboardWrapper.test.tsx
        modified:   superset-frontend/src/dashboard/components/DashboardBuilder/DashboardWrapper.tsx
        modified:   superset-frontend/src/dashboard/components/DashboardBuilder/state.ts
        modified:   superset-frontend/src/explore/actions/datasourcesActions.ts
        modified:   superset-frontend/src/explore/components/controls/ViewQuery.tsx
        new file:   superset-frontend/src/types/jest-styled-components.d.ts
        modified:   superset-frontend/tsconfig.json

In parallel:

Sequential:

add require_serial: true to type-checking-frontend hook

NOTES

Running pre-commit run type-checking-frontend --all-files is slower than running npm run type.

This is expected, due to command line length limits, it's not feasible to pass all files to a single command invocation. Instead, pre-commit splits the files into manageable chunks and processes them. (mentioned in this issue)

And superset-frontend has over 2,000 files, running all files will cause the hook to split the files into two chunks(according to test), and run sequentially. This makes sense when you compare the execution time, which is around 1 minute and 30 seconds."

Additionally, in the CI pipeline, type-checking is performed using npm run type, not pre-commit run --all-files. (refer to .github/workflows/pre-commit.yml and .github/workflows/superset-frontend.yml)

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

Has associated issue:
Required feature flags:
Changes UI
Includes DB Migration (follow approval process in SIP-59)
- Migration is atomic, supports rollback & is backwards-compatible
- Confirm DB migration upgrade and downgrade tested
- Runtime estimates and downtime expectations provided
Introduces new feature or API
Removes existing feature or API

korbit-ai

I've completed my review and didn't find any issues.

Need a new review? Comment /korbit-review on this PR and I'll review your latest changes.

Korbit Guide: Usage and Customization

Interacting with Korbit

You can manually ask Korbit to review your PR using the /korbit-review command in a comment at the root of your PR.

You can ask Korbit to generate a new PR description using the /korbit-generate-pr-description command in any comment on your PR.

Too many Korbit comments? I can resolve all my comment threads if you use the /korbit-resolve command in any comment on your PR.

Chat with Korbit on issues we post by tagging @korbit-ai in your reply.

Help train Korbit to improve your reviews by giving a 👍 or 👎 on the comments Korbit posts.

Customizing Korbit

Check out our docs on how you can make Korbit work best for you and your team.

Customize Korbit for your organization through the Korbit Console.

Current Korbit Configuration

General Settings

Setting Value

Review Schedule Automatic excluding drafts

Max Issue Count 10

Automatic PR Descriptions ❌

Issue Categories

Category Enabled

Documentation ✅

Logging ✅

Error Handling ✅

Readability ✅

Design ✅

Performance ✅

Security ✅

Functionality ✅

Feedback and Support

Tell us what you think of Korbit

Schedule a call with our team

Email us @ [email protected]

Note

Korbit Pro is free for open source projects 🎉

Looking to add Korbit to your team? Get started with a free 2 week trial here

hainenber · 2025-02-21T11:32:42Z

LGTM. Can confirm it runs much faster on 1 of my large-scale refactoring PR 👍

ci(type-checking): run type-checking-frontend hook sequentially

4d9e176

pull-request-size bot added the size/XS label Feb 20, 2025

korbit-ai bot reviewed Feb 20, 2025

View reviewed changes

sadpandajoe requested review from mistercrunch and rusackas February 20, 2025 18:28

hainenber approved these changes Feb 21, 2025

View reviewed changes

mistercrunch merged commit 5140250 into apache:master Feb 21, 2025
48 checks passed

alveifbklsiu259 mentioned this pull request Feb 22, 2025

fix(eslint-hook): ensure eslint hook receives arguments #32333

Merged

9 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ci(type-checking): run type-checking-frontend hook sequentially #32323

ci(type-checking): run type-checking-frontend hook sequentially #32323

alveifbklsiu259 commented Feb 20, 2025 •

edited

Loading

korbit-ai bot left a comment •

edited

Loading

hainenber commented Feb 21, 2025

Setting	Value
Review Schedule	Automatic excluding drafts
Max Issue Count	10
Automatic PR Descriptions	❌

Category	Enabled
Documentation	✅
Logging	✅
Error Handling	✅
Readability	✅
Design	✅
Performance	✅
Security	✅
Functionality	✅

ci(type-checking): run type-checking-frontend hook sequentially #32323

ci(type-checking): run type-checking-frontend hook sequentially #32323

Conversation

alveifbklsiu259 commented Feb 20, 2025 • edited Loading

SUMMARY

NOTES

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

ADDITIONAL INFORMATION

korbit-ai bot left a comment • edited Loading

Choose a reason for hiding this comment

Interacting with Korbit

Customizing Korbit

Current Korbit Configuration

Feedback and Support

Looking to add Korbit to your team? Get started with a free 2 week trial here

hainenber commented Feb 21, 2025

alveifbklsiu259 commented Feb 20, 2025 •

edited

Loading

korbit-ai bot left a comment •

edited

Loading