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): don't serve codeframes for files outside of compilation #38059

Merged
merged 2 commits into from
May 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion e2e-tests/development-runtime/SHOULD_NOT_SERVE
Original file line number Diff line number Diff line change
@@ -1 +1 @@
this file shouldn't be allowed to be served
this file shouldn't be allowed to be served. CYPRESS-MARKER
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
const cwd = Cypress.config(`projectRoot`)

describe(`overlay handlers don't serve unrelated files`, () => {
it(`__file-code-frame`, () => {
cy.request(
`__file-code-frame?filePath=${cwd}/SHOULD_NOT_SERVE&lineNumber=0`
).should(response => {
expect(response.body.codeFrame).not.to.match(/CYPRESS-MARKER/)
})
})

it(`__original-stack-frame`, () => {
cy.request(
`__original-stack-frame?moduleId=${cwd}/SHOULD_NOT_SERVE&lineNumber=0&skipSourceMap=1`
).should(response => {
expect(response.body.codeFrame).not.to.match(/CYPRESS-MARKER/)
expect(response.body.sourceContent).not.to.match(/CYPRESS-MARKER/)
})
})
})
5 changes: 5 additions & 0 deletions packages/gatsby/src/commands/build-html.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import type { ISlicePropsEntry } from "../utils/worker/child/render-html"
import { getPageMode } from "../utils/page-mode"
import { extractUndefinedGlobal } from "../utils/extract-undefined-global"
import { modifyPageDataForErrorMessage } from "../utils/page-data"
import { setFilesFromDevelopHtmlCompilation } from "../utils/webpack/utils/is-file-inside-compilations"

type IActivity = any // TODO

Expand Down Expand Up @@ -218,6 +219,10 @@ const doBuildRenderer = async (
)
}

if (stage === `develop-html`) {
setFilesFromDevelopHtmlCompilation(stats.compilation)
}

// render-page.js is hard coded in webpack.config
return {
rendererPath: `${directory}/${ROUTES_DIRECTORY}render-page.js`,
Expand Down
35 changes: 33 additions & 2 deletions packages/gatsby/src/utils/start-server.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import webpackHotMiddleware from "@gatsbyjs/webpack-hot-middleware"
import webpackDevMiddleware from "webpack-dev-middleware"
import got, { Method } from "got"
import webpack from "webpack"
import webpack, { Compilation } from "webpack"
import express from "express"
import compression from "compression"
import { createHandler as createGraphqlEndpointHandler } from "graphql-http/lib/use/express"
Expand Down Expand Up @@ -50,6 +50,7 @@ import { getPageMode } from "./page-mode"
import { configureTrailingSlash } from "./express-middlewares"
import type { Express } from "express"
import { addImageRoutes } from "gatsby-plugin-utils/polyfill-remote-file"
import { isFileInsideCompilations } from "./webpack/utils/is-file-inside-compilations"

type ActivityTracker = any // TODO: Replace this with proper type once reporter is typed

Expand Down Expand Up @@ -413,6 +414,19 @@ export async function startServer(
store.getState().program.directory,
req.query.moduleId as string
)

const compilation: Compilation =
res.locals?.webpack?.devMiddleware?.stats?.compilation
if (!compilation) {
res.json(emptyResponse)
return
}

if (!isFileInsideCompilations(absolutePath, compilation)) {
res.json(emptyResponse)
return
}

try {
sourceContent = fs.readFileSync(absolutePath, `utf-8`)
} catch (e) {
Expand Down Expand Up @@ -540,7 +554,24 @@ export async function startServer(
return
}

const sourceContent = await fs.readFile(filePath, `utf-8`)
const absolutePath = path.resolve(
store.getState().program.directory,
filePath
)

const compilation: Compilation =
res.locals?.webpack?.devMiddleware?.stats?.compilation
if (!compilation) {
res.json(emptyResponse)
return
}

if (!isFileInsideCompilations(absolutePath, compilation)) {
res.json(emptyResponse)
return
}

const sourceContent = await fs.readFile(absolutePath, `utf-8`)

const codeFrame = codeFrameColumns(
sourceContent,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
import { Compilation, NormalModule } from "webpack"

const filesInsideDevelopHtmlCompilation = new Set<string>()

function removeQueryParams(path: string): string {
return path.split(`?`)[0]
}

export function setFilesFromDevelopHtmlCompilation(
developHtmlCompilation: Compilation
): void {
filesInsideDevelopHtmlCompilation.clear()

for (const module of developHtmlCompilation.modules) {
if (module instanceof NormalModule && module.resource) {
filesInsideDevelopHtmlCompilation.add(removeQueryParams(module.resource))
}
}
}

/**
* Checks if a file is inside either `develop` or `develop-html` compilation. Used to determine if
* we should generate codeframe for this file for error overlay.
*/
export function isFileInsideCompilations(
absolutePath: string,
developBrowserCompilation: Compilation
): boolean {
if (filesInsideDevelopHtmlCompilation.has(absolutePath)) {
return true
}

for (const module of developBrowserCompilation.modules) {
if (module instanceof NormalModule && module.resource) {
if (absolutePath === removeQueryParams(module.resource)) {
return true
}
}
}

return false
}