Skip to content
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(gatsby): Fix various small DEV_SSR bugs exposed in development_runtime tests #29720

Merged
merged 13 commits into from
Mar 1, 2021

Conversation

KyleAMathews
Copy link
Contributor

@KyleAMathews KyleAMathews commented Feb 23, 2021

PR includes some misc test fixes to fixes issues in tests.

@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Feb 23, 2021
@@ -126,7 +126,7 @@ apiRunnerAsync(`onClientEntry`).then(() => {
undefined,
// Client only pages have any empty body so we just do a normal
// render to avoid React complaining about hydration mis-matches.
document.getElementById(`___gatsby`).children.length === 0
document.getElementById(`gatsby-focus-wrapper`).children.length === 0
Copy link
Contributor Author

Choose a reason for hiding this comment

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

___gatsby always has a child so wasn't actually testing if there was SSRed content or not so client-only pages would show a hydration warning.

@@ -10,13 +10,13 @@ export const route = ({ app, program, store }): any =>
app.get(`*`, async (req, res, next) => {
trackFeatureIsUsed(`GATSBY_EXPERIMENTAL_DEV_SSR`)

const pathObj = findPageByPath(store.getState(), req.path)
const pathObj = findPageByPath(store.getState(), decodeURI(req.path))
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Handle paths with unicode characters

@@ -59,8 +59,13 @@ export const restartWorker = (htmlComponentRendererPath): void => {

const searchFileForString = (substring, filePath): Promise<boolean> =>
new Promise(resolve => {
const escapedSubString = substring.replace(/[.*+?^${}()|[\]\\]/g, `\\$&`)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Handle paths with special characters

@KyleAMathews KyleAMathews added topic: ssr and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Feb 24, 2021
@KyleAMathews KyleAMathews changed the title feat(gatsby): Enable DEV_SSR for everyone feat(gatsby): Fix various small DEV_SSR bugs exposed in development_runtime tests Feb 24, 2021
@KyleAMathews KyleAMathews changed the title feat(gatsby): Fix various small DEV_SSR bugs exposed in development_runtime tests fix(gatsby): Fix various small DEV_SSR bugs exposed in development_runtime tests Feb 24, 2021
@pieh pieh merged commit 114e006 into master Mar 1, 2021
@pieh pieh deleted the dev-ssr-100% branch March 1, 2021 13:44
vladar pushed a commit that referenced this pull request Mar 1, 2021
…ntime tests (#29720)

* the dev server now returns 404s which would otherwise fail the tests

* Correctly detect if there's any SSRed HTML as ___gatsby always has the focus wrapper child

* Handle unicode paths like /안녕

* Properly escape paths with special characters in it

* Enable DEV_SSR for everyone

* Fix status in test

* Revert to 20% rollout

* revert changes for 100% rollout

* test this

(cherry picked from commit 114e006)
vladar pushed a commit that referenced this pull request Mar 1, 2021
…ntime tests (#29720) (#29866)

* the dev server now returns 404s which would otherwise fail the tests

* Correctly detect if there's any SSRed HTML as ___gatsby always has the focus wrapper child

* Handle unicode paths like /안녕

* Properly escape paths with special characters in it

* Enable DEV_SSR for everyone

* Fix status in test

* Revert to 20% rollout

* revert changes for 100% rollout

* test this

(cherry picked from commit 114e006)

Co-authored-by: Kyle Mathews <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: DX Developer Experience (e.g. Fast Refresh, i18n, SSR, page creation, starters)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants