Skip to content

Commit

Permalink
fix(gatsby): don't serve codeframes for files outside of compilation (#…
Browse files Browse the repository at this point in the history
…38059) (#38062)

* test: add test case for overlay handlers

* fix: don't serve codeframes for files outside of compilation

(cherry picked from commit ed5855e)

Co-authored-by: Michal Piechowiak <misiek.piechowiak@gmail.com>
  • Loading branch information
gatsbybot and pieh authored May 5, 2023
1 parent 18a47da commit ae5a654
Show file tree
Hide file tree
Showing 5 changed files with 101 additions and 3 deletions.
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
}

1 comment on commit ae5a654

@HMMM116
Copy link

Choose a reason for hiding this comment

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

....

Please sign in to comment.