Skip to content

build: streamline by removing unneeded build deps #25733

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
tylerbutler merged 4 commits into from
Oct 23, 2025
Merged

build: streamline by removing unneeded build deps #25733

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ab36426
build: streamline by removing needed build deps
tylerbutler Oct 22, 2025
08754dd
Merge branch 'main' into build-graph-streamline
tylerbutler Oct 22, 2025
c80f350
Merge branch 'main' into build-graph-streamline
tylerbutler Oct 23, 2025
80be4bb
feedback
tylerbutler Oct 23, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 5 additions & 7 deletions fluidBuild.config.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,14 @@ module.exports = {
"check:format",
"compile",
"lint",
"build:api-reports",
"build:docs",
// API validation happens through lint → check:exports → api
Copy link
Contributor

@Josmithr Josmithr Oct 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems odd to me that the lint step would need/run the API extractor steps that generate the API reports. I get the export checks being a part of lint, but I don't think the report generation steps belong there. Seems like they should be a dependency of build and not of lint.

Copy link
Contributor

@CraigMacomber CraigMacomber Oct 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing it here seems like a hack to make it skipped when we run with --nolint by making everything that depends on it part of lint.

Removing it from compile below should be sufficient to relax the build dependency order and remove it from --task compile builds.

Leaving it here might be better.

Copy link
Contributor

@CraigMacomber CraigMacomber Oct 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should make out compile CI task run just the compile step, then have a separate job (in parallel with tests) that run everything else, which from this.

That suggests maybe there is some logical grouping includes all of "check:format", "lint", "build:api-reports", "build:docs", "build:manifest", "build:readme".

I could see grouping "check:format" into lint, and the rest into "build:docs" if we wanted less top level categories here.

Copy link
Member Author

@tylerbutler tylerbutler Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should make out compile CI task run just the compile step, then have a separate job (in parallel with tests) that run everything else, which from this.

That suggests maybe there is some logical grouping includes all of "check:format", "lint", "build:api-reports", "build:docs", "build:manifest", "build:readme".

I could see grouping "check:format" into lint, and the rest into "build:docs" if we wanted less top level categories here.

If we're talking task naming and grouping overall, I suggest using this PR that has some concrete all-up proposals: #25730 (comment)

Copy link
Member Author

@tylerbutler tylerbutler Oct 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems odd to me that the lint step would need/run the API extractor steps that generate the API reports. I get the export checks being a part of lint, but I don't think the report generation steps belong there. Seems like they should be a dependency of build and not of lint.

Fixed in latest.

"build:manifest",
"build:readme",
],
script: false,
},
"compile": {
dependsOn: ["commonjs", "build:esnext", "api", "build:test", "build:copy"],
dependsOn: ["commonjs", "build:esnext", "build:test", "build:copy"],
script: false,
},
"commonjs": {
Expand Down Expand Up @@ -77,12 +76,11 @@ module.exports = {
// Generic build:test script should be replaced by :esm or :cjs specific versions.
// "tsc" would be nice to eliminate from here, but plenty of packages still focus
// on CommonJS.
"build:test": ["typetests:gen", "tsc", "api-extractor:commonjs", "api-extractor:esnext"],
"build:test:cjs": ["typetests:gen", "tsc", "api-extractor:commonjs"],
"build:test:esm": ["typetests:gen", "build:esnext", "api-extractor:esnext"],
"build:test": ["typetests:gen", "tsc"],
"build:test:cjs": ["typetests:gen", "tsc"],
"build:test:esm": ["typetests:gen", "build:esnext"],
"api": {
dependsOn: ["api-extractor:commonjs", "api-extractor:esnext"],
// dependsOn: ["api-extractor:commonjs", "api-extractor:esnext"],
script: false,
},
"api-extractor:commonjs": ["tsc"],
Expand Down
Loading