-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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
feat(gatsby): New overlay for DEV_SSR #31061
Conversation
@@ -38,7 +38,7 @@ describe(`SSR`, () => { | |||
const pageUrl = `http://localhost:8000/bad-page/` | |||
// Poll until the new page is bundled (so starts returning a non-404 status). | |||
const rawDevHtml = await fetchUntil(pageUrl, res => { | |||
return res.status === 500 | |||
return res |
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.
We don't explicitly send 500
status anymore as other errors (runtime, build, graphql) also don't do that
line ? `:${line}:${column}` : `` | ||
} to resolve the error.`, | ||
text: (context): string => | ||
stripIndent(`The path "${context.path}" errored during SSR. |
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.
Since it's a WARNING
the docsUrl
is unused. Also added stripIndent
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.
stripIndent
won't do anything here because it looks for shortest indentation and removes that from every line - in here shortest indentation is empty string (because first line doesn't have any indentation)
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.
above comment addressed in 9eb5a5b
export function prettifyStack(errorInformation) { | ||
let txt | ||
if (Array.isArray(errorInformation)) { | ||
txt = errorInformation.join(`\n`) | ||
} else { | ||
txt = errorInformation | ||
} | ||
const generated = Anser.ansiToJson(txt, { | ||
return Anser.ansiToJson(txt, { |
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 was seeing weird behavior with removing the first line for all different errors. The underlying error format must have improved so this isn't necessary anymore
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.
🤞 This doesn't give me much confidence 😛 Maybe it happened because of #30801
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 was seeing weird behavior with removing the first line for all different errors.
Should rephrase that: The indentation for the now first line was slightly incorrect as the previously existing new line wasn't there anymore.
The PR you linked might have caused this, yeah
|
||
const startWorker = (): any => { | ||
const startWorker = (): JestWorker => { |
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.
Fixing TS errors in this file
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.
Do you want to add e2e tests so the buttons like refresh & skipSSR do actually work?
@@ -5,34 +5,38 @@ Cypress.on(`window:before:load`, win => { | |||
|
|||
const runTests = () => { | |||
it(`should redirect page to index page when there is no such page`, () => { | |||
cy.visit(`/redirect-without-page`).waitForRouteChange() | |||
cy.visit(`/redirect-without-page`, { | |||
failOnStatusCode: false, |
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 left these in as in the future when we'd send 404
for the not found page those would fail
Description
This is for
DEV_SSR
flag (Discussion: #28138)Previously the error overlay (when the page didn't successfully ssr'ed) consisted out of a HTML page served by express with helpful information. But that wasn't tied into our already existing Fast Refresh overlay we use throughout Gatsby.
Instead of serving all the information in the HTML a client-only shell is served that pushes to the
window._gatsbyEvents
queue.Screenshots
[ch29833]