-
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
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.
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
fix: Add fallback for spa in preview. #10944
Conversation
30f4e79
to
38f919a
Compare
We would need to update the failing tests too. Looks like 2 we're incorrectly checking this for the SPA case. |
I took some stabs at updating the tests. Currently it defaults to being an appType of |
18901c5
to
a56c139
Compare
7e03965
to
5136f11
Compare
@@ -70,6 +71,7 @@ describe.runIf(isServe)('serve', () => { | |||
}) | |||
|
|||
test('CSS dependencies are tracked for HMR', async () => { | |||
await startDefaultServe('spa') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not 100% sure why this is necessary.
'index: 404', | ||
'index-legacy: 404', | ||
'chunk-async: 404', | ||
'chunk-async-legacy: 404', | ||
'immutable-chunk: 404', | ||
'immutable-chunk-legacy: 404', | ||
'polyfills-legacy: 404' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure why this would be 404s?
I don't understand why my tests fail on Linux but not on macOS |
Description
fix #10925
Resolves this issue - Routing broken for preview with react-router-dom in 4.0.0-alpha.2
When
config.appType === 'spa'
is true then then the single option of sirv is enabled. Docs - "When true, the directory's index page (default index.html) will be sent if the request asset does not exist."So, I think we would want to mimic the behavior of sirv for spa apps and to fallback to routing to index.html when shouldServe is false.
Questions - is there an appropriate way to handle the tests for this? It's blowing up.
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).