feat: add spa_fallback config option - #7141
Merged
Merged
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds the optional Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Contributor
|
This pull request adds or modifies JavaScript ( |
Contributor
e18e dependency analysisNo dependency warnings found. |
@netlify/build
@netlify/build-info
@netlify/cache-utils
@netlify/config
@netlify/edge-bundler
@netlify/functions-utils
@netlify/git-utils
@netlify/headers-parser
@netlify/api
@netlify/nock-udp
@netlify/opentelemetry-sdk-setup
@netlify/opentelemetry-utils
@netlify/redirect-parser
@netlify/run-utils
@netlify/zip-it-and-ship-it
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/build/tests/frameworks_api/tests.js`:
- Line 99: Update the Fixture construction in the test to pass test.meta.file as
the first argument and './fixtures/spa_config' as the second, ensuring the
fixture resolves its repository root and uses the intended SPA configuration.
In `@packages/config/tests/validate/tests.js`:
- Around line 121-124: Update the build.spa boolean test’s Fixture construction
to pass the current test file metadata via test.meta.file along with the
relative fixture path, matching the two-argument form used by surrounding tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 579eefc1-8f7a-422c-b4d2-ce6eea359b85
⛔ Files ignored due to path filters (4)
packages/build/tests/core/snapshots/tests.js.snapis excluded by!**/*.snappackages/build/tests/telemetry/snapshots/tests.js.snapis excluded by!**/*.snappackages/build/tests/time/snapshots/tests.js.snapis excluded by!**/*.snappackages/config/tests/validate/snapshots/tests.js.snapis excluded by!**/*.snap
📒 Files selected for processing (25)
eslint_temporary_suppressions.jspackages/build/src/plugins_core/frameworks_api/index.tspackages/build/src/plugins_core/spa_fallback/index.tspackages/build/src/steps/get.tspackages/build/src/types/config/build.tspackages/build/tests/core/snapshots/tests.js.mdpackages/build/tests/frameworks_api/fixtures/spa_config/.gitignorepackages/build/tests/frameworks_api/fixtures/spa_config/build.mjspackages/build/tests/frameworks_api/fixtures/spa_config/netlify.tomlpackages/build/tests/frameworks_api/tests.jspackages/build/tests/spa_fallback/fixtures/spa_default/netlify.tomlpackages/build/tests/spa_fallback/fixtures/spa_disabled/netlify.tomlpackages/build/tests/spa_fallback/fixtures/spa_enabled/netlify.tomlpackages/build/tests/spa_fallback/fixtures/spa_enabled_existing_catch_all/netlify.tomlpackages/build/tests/spa_fallback/tests.jspackages/build/tests/telemetry/snapshots/tests.js.mdpackages/build/tests/time/snapshots/tests.js.mdpackages/config/src/case.tspackages/config/src/context.tspackages/config/src/mutations/apply.jspackages/config/src/validate/helpers.jspackages/config/src/validate/validations.jspackages/config/tests/validate/fixtures/build_spa_boolean/netlify.tomlpackages/config/tests/validate/snapshots/tests.js.mdpackages/config/tests/validate/tests.js
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
netlify/blueprints(manual)
serhalp
reviewed
Jul 24, 2026
serhalp
left a comment
serhalp
left a comment
Member
There was a problem hiding this comment.
Looks great!
Couple minor notes inline and let's settle the naming question on Slack 👀
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
packages/build/tests/frameworks_api/spa.test.ts (1)
4-12: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winCover the
netlify.tomlprecedence contract.This test proves that the Frameworks API can set
build.spa, but not that a conflictingnetlify.tomlvalue wins. Add a fixture with opposite values from both sources and assert the normalized value followsnetlify.toml, or confirm equivalent coverage exists elsewhere.As per the PR objective,
netlify.tomltakes precedence over the Frameworks API.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/build/tests/frameworks_api/spa.test.ts` around lines 4 - 12, Extend the SPA fixture coverage around the “Honors `build.spa` declared through the Frameworks API config file” test to provide conflicting `build.spa` values from the Frameworks API config and netlify.toml. Assert the normalized netlifyConfig.build.spa matches the netlify.toml value, confirming it takes precedence over the Frameworks API.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@eslint_temporary_suppressions.js`:
- Around line 2848-2858: Reformat the files array in the suppression block by
placing each path on its own line with the project’s standard multiline array
formatting, so eslint_temporary_suppressions.js passes oxfmt --check. Leave the
suppression rules and file entries unchanged.
In `@packages/build/tsconfig.build.json`:
- Around line 9-10: Update the include/exclude configuration in
tsconfig.build.json so source test files matching src/**/*.test.ts are excluded
from the production build. Preserve inclusion of non-test JavaScript and
TypeScript source files and the existing tests/** exclusion.
In `@packages/build/tsconfig.json`:
- Line 7: Remove the ./tsconfig.test.json entry from the project references in
packages/build/tsconfig.json, leaving ./tsconfig.build.json referenced. In
packages/build/tsconfig.test.json, make no direct change; it should remain a
standalone test configuration unless intentionally converted into an emitting
composite project.
---
Nitpick comments:
In `@packages/build/tests/frameworks_api/spa.test.ts`:
- Around line 4-12: Extend the SPA fixture coverage around the “Honors
`build.spa` declared through the Frameworks API config file” test to provide
conflicting `build.spa` values from the Frameworks API config and netlify.toml.
Assert the normalized netlifyConfig.build.spa matches the netlify.toml value,
confirming it takes precedence over the Frameworks API.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f9aa2d2c-a61f-44b6-9337-a31c1b4e6db1
📒 Files selected for processing (12)
eslint_temporary_suppressions.jspackages/build/.gitignorepackages/build/package.jsonpackages/build/src/plugins/child/diff.tspackages/build/src/plugins_core/spa_fallback/index.tspackages/build/tests/frameworks_api/spa.test.tspackages/build/tests/spa_fallback/spa_fallback.test.tspackages/build/tsconfig.build.jsonpackages/build/tsconfig.jsonpackages/build/tsconfig.test.jsonpackages/build/vite.config.tstsconfig.base.json
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
netlify/blueprints(manual)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/build/src/plugins_core/spa_fallback/index.ts
hrishikesh-k
changed the title
serhalp
approved these changes
Aug 4, 2026
Merged
5 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Update: Changed
build.spatospa_fallbackThis PR introduces a new config option in
netlify.tomland in Frameworks API (and also in build plugins):build.spa(boolean, default: false). Innetlify.toml, it looks like:In Frameworks API (
config.json), it looks like:{ "build": { "spa": true } }netlify.tomltakes preference over Frameworks API. Once this property is set totrue, it would inject the default SPA rewrite (/* /index.html 200) that's required for SPAs. Since this is done as a part of a core build step, it works in the CLI as well as in CI, whilenetlify devremains unaffected.