feat: CT stack traces #23916
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -105,6 +105,27 @@ export const Cypress = ( | |
| return res.end(transformedIndexHtml) | ||
| }) | ||
| }, | ||
| transform (code, id, options?) { | ||
| try { | ||
| if (/\.js$/i.test(id) && !/\/\/# sourceMappingURL=/i.test(code)) { | ||
| // The Vite dev server and plugins automatically transpile TS and JSX files, which results in sourcemaps being generated | ||
| // and included in output. However, Vite serves up `esnext`-compliant Javascript which means many JS files won't be | ||
| // transpiled and won't supply a sourcemap - this prevents Cypress from providing codeFrames in the event of an error. | ||
| // | ||
| // A sourcemap is generated by Vite for JS files (just not included) which is in effect an "identity" sourcemap mapping | ||
| // 1-to-1 to the output file. We can grab this and pass it along as a sourcemap we want Vite to embed into the output, | ||
| // giving Cypress a sourcemap to use for codeFrame lookups. | ||
| // @see https://rollupjs.org/guide/en/#thisgetcombinedsourcemap | ||
|
|
||
| return { | ||
| code, | ||
| map: this.getCombinedSourcemap(), | ||
|
There was a problem hiding this comment. Right, took me a bit to realize this is from the underyling Rollup API https://rollupjs.org/guide/en/#thisgetcombinedsourcemap According to the Vite docs There was a problem hiding this comment. Edit, apparently not - as per your comment above, we do need to call this function, but I don't quite know why. I still think I'm missing some understanding here. If you write something like: interface Blah {
foo: string
}
const blah = (): Blah => {
throw Error('argh')
return { foo: "bar" }
}
blah()And serve via Vite - it strips out TS, but this would require a sourcemap to get correct error location - if Vite doesn't generate sourcemaps, how does this error manifest in a regular Vite dev env? I'll try it out. Hm. There was a problem hiding this comment. Seems Vite is generating a sourcemap normally. I wonder why we need to do this - why can't we do
There was a problem hiding this comment. @lmiller1990 I think it comes down to the difference between serving a "pure" JS file vs a typescript or JSX sort of file. The Vite has to transpile TS and JSX files so a sourcemap is appended automatically, but for JS files that don't require transpilation (just served up as-is) the sourcemap was not getting appended unless I added that explicit call. There was a problem hiding this comment. Oh right, so you are saying it will automatically append the sourceMap for anything other than plain JS... this is only required for the case of plain JS (eg, does it "just work" for TSX, for example, even if I remove this?) This is my understanding based on your comment. I will try this to find out if my assumption is correct. There was a problem hiding this comment. @lmiller1990 Correct - this is here to handle the case where source JS files would not get sourcemaps. Pretty much everything else did without this change There was a problem hiding this comment. Makes sense, since normally you don't need source maps for JS - nothing to map 🎉 This worked out surprisingly well, glad Vite exposes the Rollup API. |
||
| } | ||
| } | ||
| } catch (_err) { | ||
| debug('Failed to propagate sourcemap for %s: %o', id, _err) | ||
| } | ||
| }, | ||
| handleHotUpdate: ({ server, file }) => { | ||
| debug('handleHotUpdate - file', 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.
Not related to this file but @brian-mann pointed out we will also want to run
As well, but with CT.
These are the ones that verify the typescript compiler is correctly monkey patched to generate the correct source maps in E2E when using webpack-preprocessor.
The monkey patch is here: https://github.com/cypress-io/cypress/blob/develop/npm/webpack-preprocessor/lib/typescript-overrides.ts#L39-L45
It's possible this is actually not required. if {webpack,vite}-dev-server already do this (or something like this). We can find out by running these tests in CT, and see if they pass. If they don't, we might need something similar.
Ref: https://cypressio.slack.com/archives/C011BAPC614/p1662579814502909
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 am working this in another branch to avoid complicating this PR any further. Will open as a follow-on PR in case there are complications so we don't block the main stacktraces featureset from release - seems to be working as expected once I ported the patch logic over except for a special case for Angular
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.
Ok, sounds good