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

Incorrect paths in sources? #9

Closed
LekoArts opened this issue May 11, 2022 · 6 comments · Fixed by #10
Closed

Incorrect paths in sources? #9

LekoArts opened this issue May 11, 2022 · 6 comments · Fixed by #10

Comments

@LekoArts
Copy link

LekoArts commented May 11, 2022

Hi, it's me again :)

For my current WIP PR gatsbyjs/gatsby#35621 our E2E tests fail (correctly, they caught a bug). The result in our CLI is that the codeFrame is not rendered. You don't have to have the context on Gatsby, I think I can explain the problem also without that knowledge.

So I'm using the new sourceContentFor here:

https://github.com/gatsbyjs/gatsby/blob/source-map-migration/packages/gatsby-cli/src/reporter/prepare-stack-trace.ts#L73

It receives a map and topFrame.getFileName() which comes from here:

https://github.com/gatsbyjs/gatsby/blob/source-map-migration/packages/gatsby-cli/src/reporter/prepare-stack-trace.ts#L47-L59

So we're using stack-trace to parse the stacktrace, it creates a stack and we're using the first item of that stack as a "topFrame".

Now here comes the problem:

The getFileName() from stack-trace is different to the sources inside this library. When I log out map.sources and topFrame.getFileName() inside the loop, I get:

{
  sources: [
    'webpack://source-map-testing/./node_modules/@babel/runtime/helpers/assertThisInitialized.js',
    'webpack://source-map-testing/./node_modules/@babel/runtime/helpers/extends.js',
    'webpack://source-map-testing/./node_modules/@babel/runtime/helpers/inheritsLoose.js',
    'webpack://source-map-testing/./node_modules/@babel/runtime/helpers/interopRequireDefault.js',
    'webpack://source-map-testing/./node_modules/@babel/runtime/helpers/objectWithoutPropertiesLoose.js',
    'webpack://source-map-testing/./node_modules/@babel/runtime/helpers/setPrototypeOf.js',
    'webpack://source-map-testing/./node_modules/@gatsbyjs/reach-router/lib/utils.js',
    'webpack://source-map-testing/./node_modules/deepmerge/dist/cjs.js',
    'webpack://source-map-testing/./node_modules/gatsby-link/index.js',
    'webpack://source-map-testing/./node_modules/gatsby-link/is-local-link.js',
    'webpack://source-map-testing/./node_modules/gatsby-link/parse-path.js',
    'webpack://source-map-testing/./node_modules/gatsby-link/rewrite-link-path.js',
    'webpack://source-map-testing/./node_modules/gatsby-page-utils/dist/apply-trailing-slash-option.js',
    'webpack://source-map-testing/./node_modules/gatsby-react-router-scroll/index.js',
    'webpack://source-map-testing/./node_modules/gatsby-react-router-scroll/scroll-handler.js',
    'webpack://source-map-testing/./node_modules/gatsby-react-router-scroll/session-storage.js',
    'webpack://source-map-testing/./node_modules/gatsby-react-router-scroll/use-scroll-restoration.js',
    'webpack://source-map-testing/./.cache/_this_is_virtual_fs_path_/$virtual/async-requires.js',
    'webpack://source-map-testing/./.cache/api-runner-ssr.js',
    'webpack://source-map-testing/./.cache/api-ssr-docs.js',
    'webpack://source-map-testing/./.cache/default-html.js',
    'webpack://source-map-testing/./.cache/strip-prefix.js',
    'webpack://source-map-testing/./.cache/normalize-page-path.js',
    'webpack://source-map-testing/./.cache/redirect-utils.js',
    'webpack://source-map-testing/./.cache/find-path.js',
    'webpack://source-map-testing/./.cache/prefetch.js',
    'webpack://source-map-testing/./node_modules/mitt/dist/mitt.es.js',
    'webpack://source-map-testing/./.cache/emitter.js',
    'webpack://source-map-testing/./.cache/loader.js',
    'webpack://source-map-testing/./.cache/gatsby-browser-entry.js',
    'webpack://source-map-testing/./.cache/public-page-renderer.js',
    'webpack://source-map-testing/./.cache/react-lifecycles-compat.js',
    'webpack://source-map-testing/./.cache/route-announcer-props.js',
    'webpack://source-map-testing/external commonjs "stream"',
    'webpack://source-map-testing/./.cache/server-utils/writable-as-promise.js',
    'webpack://source-map-testing/./gatsby-ssr.js',
    'webpack://source-map-testing/./node_modules/@gatsbyjs/reach-router/es/lib/utils.js',
    'webpack://source-map-testing/./node_modules/@gatsbyjs/reach-router/es/lib/history.js',
    'webpack://source-map-testing/./node_modules/@gatsbyjs/reach-router/es/index.js',
    'webpack://source-map-testing/./node_modules/invariant/invariant.js',
    'webpack://source-map-testing/./node_modules/prop-types/factoryWithThrowingShims.js',
    'webpack://source-map-testing/./node_modules/prop-types/index.js',
    'webpack://source-map-testing/./node_modules/prop-types/lib/ReactPropTypesSecret.js',
    'webpack://source-map-testing/external commonjs
"/Users/lejoe/code/playground/source-map-testing/.cache/ssr-builtin-trackers/fs"',
    'webpack://source-map-testing/external commonjs
"/Users/lejoe/code/playground/source-map-testing/node_modules/react-dom/server.js"',
    'webpack://source-map-testing/external commonjs "/Users/lejoe/code/playground/source-map-testing/node_modules/react/index.js"',
    'webpack://source-map-testing/external commonjs "path"',
    'webpack://source-map-testing/webpack/bootstrap',
    'webpack://source-map-testing/webpack/runtime/compat get default export',
    'webpack://source-map-testing/webpack/runtime/define property getters',
    'webpack://source-map-testing/webpack/runtime/ensure chunk',
    'webpack://source-map-testing/webpack/runtime/get javascript chunk filename',
    'webpack://source-map-testing/webpack/runtime/hasOwnProperty shorthand',
    'webpack://source-map-testing/webpack/runtime/make namespace object',
    'webpack://source-map-testing/webpack/runtime/require chunk loading',
    'webpack://source-map-testing/./.cache/static-entry.js'
  ],
  topFrame: {
    getFileName: [Function: getFileName],
    getLineNumber: [Function: getLineNumber],
    getColumnNumber: [Function: getColumnNumber],
    getScriptNameOrSourceURL: [Function: getScriptNameOrSourceURL],
    toString: [Function: CallSiteToString],
    wasConverted: true
  },
  fileName: 'webpack://source-map-testing/gatsby-ssr.js'
}

So:

'webpack://source-map-testing/gatsby-ssr.js' !== 'webpack://source-map-testing/./gatsby-ssr.js'

I think this is a bug inside this library, would you agree?

@jridgewell
Copy link
Owner

jridgewell commented May 11, 2022

I'm afraid I can't install the node modules on my M1 MPB, so I can't run the test suite. I have an idea of what's gone wrong. Can you try running yarn add 'jridgewell/trace-mapping#resolved-sources-no-map-or-root' (which is the resolved-sources-no-map-or-root branch) to see if the fix works?

@LekoArts
Copy link
Author

Sure, no problem. I'll give it a try 👍

@LekoArts
Copy link
Author

LekoArts commented May 11, 2022

Yes, this fixes it 👍
When I logged out source from https://github.com/gatsbyjs/gatsby/blob/source-map-migration/packages/gatsby-cli/src/reporter/prepare-stack-trace.ts#L73 previously I only got null.

I now get:

{
  source: '// This will cause an error during SSR because window is not defined\n' +
    'exports.onPreRenderHTML = () => {\n' +
    '  window.location.pathname\n' +
    '}\n'
}

Which is what I expect.

So the sourceContentFor works now. (The paths inside sources stayed the same)

@jridgewell
Copy link
Owner

Ok, I can release a new version in about 2 hours.

@LekoArts
Copy link
Author

No worries, it's not super urgent so don't feel pressured to rush this :) Release it when you have time. Thank you!

@nicolo-ribaudo
Copy link

Hi! I come here from babel/babel#14676.

I think that the fix should be reverted since it breaks other use cases, and most importantly the difference you see (the extra /./ in the paths) has nothing to do with this package. You can see that webpack generates the extra doc even without using this package: https://github.com/nicolo-ribaudo/webpack-dot-in-source-maps.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants