-
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
fix(gatsby): try to automatically recover when parcel segfaults #38773
Changes from all commits
0b5550f
be2dfe4
6360181
e21fb92
a5320af
31e7049
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,9 +18,10 @@ const dir = { | |
misnamedJS: `${__dirname}/fixtures/misnamed-js`, | ||
misnamedTS: `${__dirname}/fixtures/misnamed-ts`, | ||
gatsbyNodeAsDirectory: `${__dirname}/fixtures/gatsby-node-as-directory`, | ||
errorInCode: `${__dirname}/fixtures/error-in-code-ts`, | ||
} | ||
|
||
jest.setTimeout(15000) | ||
jest.setTimeout(60_000) | ||
|
||
jest.mock(`@parcel/core`, () => { | ||
const parcelCore = jest.requireActual(`@parcel/core`) | ||
|
@@ -175,6 +176,37 @@ describe(`gatsby file compilation`, () => { | |
}) | ||
}) | ||
}) | ||
|
||
it(`handles errors in TS code`, async () => { | ||
process.chdir(dir.errorInCode) | ||
await remove(`${dir.errorInCode}/.cache`) | ||
await compileGatsbyFiles(dir.errorInCode) | ||
|
||
expect(reporterPanicMock).toMatchInlineSnapshot(` | ||
[MockFunction] { | ||
"calls": Array [ | ||
Array [ | ||
Object { | ||
"context": Object { | ||
"filePath": "<PROJECT_ROOT>/gatsby-node.ts", | ||
"generalMessage": "Expected ';', '}' or <eof>", | ||
"hints": null, | ||
"origin": "@parcel/transformer-js", | ||
"specificMessage": "This is the expression part of an expression statement", | ||
Comment on lines
+191
to
+195
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. there is improvements to be made here to point out specific location as error we get is like this:
but it's out of scope of this PR - purpose here is to try to handle segfaults without introducing regression - this added tests is just just a test for case we didn't have (what's the behavior on ts files that are invalid) and that I did break before my most recent commit |
||
}, | ||
"id": "11901", | ||
}, | ||
], | ||
], | ||
"results": Array [ | ||
Object { | ||
"type": "return", | ||
"value": undefined, | ||
}, | ||
], | ||
} | ||
`) | ||
}) | ||
}) | ||
|
||
describe(`gatsby-node directory is allowed`, () => { | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,47 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { GatsbyNode } from "gatsby" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { working } from "../utils/say-what-ts" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { createPages } from "../utils/create-pages-ts" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
this is wrong syntax that should't compile | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this line be a comment? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh or is this there to fail the compilation 👍 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, that's test fixture to test behavior when there is error in source file that should result in compilation problem and test itself is here gatsby/packages/gatsby/src/utils/parcel/__tests__/compile-gatsby-files.ts Lines 180 to 210 in 31e7049
I did break this behavior when iterating on this code and noticed we didn't have tests for this case so I added it as part of this PR #38773 (comment) this comment highlight opportunity to improve error message possibly, but this was just out of scope of the fix |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
export const onPreInit: GatsbyNode["onPreInit"] = ({ reporter }) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
reporter.info(working) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
type Character = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
id: string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
name: string | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
export const sourceNodes: GatsbyNode["sourceNodes"] = async ({ actions, createNodeId, createContentDigest }) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const { createNode } = actions | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
let characters: Array<Character> = [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
id: `0`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
name: `A` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
id: `1`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
name: `B` | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
characters.forEach((character: Character) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const node = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
...character, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
id: createNodeId(`characters-${character.id}`), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
parent: null, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
children: [], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
internal: { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
type: 'Character', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
content: JSON.stringify(character), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
contentDigest: createContentDigest(character), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
createNode(node) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
export { createPages } |
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 is just a trick for unit tests to be able to use source TS (and ESM) files when using
gatsby-worker
/ child process setupIn actual usage TS source files become CJS and those will be used (all e2e tests would be using that for example)
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 is added in this PR and as global mock because
compile-gatsby-files
is being directly and indrectly used in quite a bit of unit tests, so to avoid hunting for all indirect usage we just allow that to happen any timegatsby-worker
is used within those tests