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

Make internal transforms source-map compliant #1213

Open
3 tasks
FredKSchott opened this issue Oct 5, 2020 · 19 comments
Open
3 tasks

Make internal transforms source-map compliant #1213

FredKSchott opened this issue Oct 5, 2020 · 19 comments
Labels
3.1 contributors welcome! contributors welcome!

Comments

@FredKSchott
Copy link
Owner

FredKSchott commented Oct 5, 2020

Now that #1048 has merged, we can add full sourcemap support. All that's needed is to convert our current dev/build string modifications to use https://www.npmjs.com/package/magic-string.

  • wrapImportMeta (all other proxy files are okay / out of scope for now, since those were not JS to begin with)
  • createImportResolver (will need to use the magic string s.overwrite API).
  • audit all official plugins to make sure that they use new source-map API in transform() methods

Also, now that we support source maps with transforms we can log a warning when sourceMap: true but a transform function isn't returning source maps, causing no maps to be created.

@FredKSchott FredKSchott added the contributors welcome! contributors welcome! label Oct 12, 2020
@alex-saunders
Copy link
Collaborator

Hey @FredKSchott ! I'm interested in picking this up, I've got some experience using magic-string with creating rollup plugins so I should have some idea of the work involved here.
Looking forward to contributing to snowpack again!

@AlonMiz
Copy link

AlonMiz commented Oct 16, 2020

I think it's pretty important.
We've tried snowpack in our project and eventually reverted cause it didn't generate source-maps while working with React (or Typescript).
I've seen you several times wondering why source-maps are important in dev mode. But, and i think it's crucial
let's consider that piece of code:
image
transpiled into this code:
image
a react developer will not be able to work with those outputs.

@David-Else
Copy link
Contributor

@FredKSchott I have been messing around with the Chrome devtools and getting nowhere, can I enable sourcemaps somehow right now even with errors? I would dearly love to stay in VS Code for debugging. I am not using any snowpack plugins, only the default TypeScript blank template. Thanks!

@gr2m
Copy link
Contributor

gr2m commented Oct 21, 2020

Hey @alex-saunders thanks for picking it up! Let us know if there is anything we can help you with. Please feel free to start a work-in-progress pull request as early as you like

@Gregoor
Copy link

Gregoor commented Nov 23, 2020

Also happy to help with this, @alex-saunders lmk if you need eyes/ears/brains for review or I can also take a stab at it if you don't have the time right now (though it sounds like you're more experienced with the subject matter).

@adam-beck
Copy link

I don't have a lot of experience with building any sort of tooling but this seems like such an important feature to include. So, as a beginner what sort of things would help me get started implementing this? I know a couple people have offered help but it seems the issue isn't making much progress.

I want to help. I just don't know what I'm doing.

@FredKSchott
Copy link
Owner Author

Discussion/Proposal created to intentionally remove sourcemap support from dev in v3.0 (while keeping it for bundled/optimized/minfied build output)
#1876

@AlonMiz
Copy link

AlonMiz commented Jan 23, 2021

just pointing out that the #1876 discussion resulted to actually implement source maps in dev mode.
I'm waiting for this feature for migrating all of my projects to Snowpack. really, this is the most important missing feature.

@jonlepage
Copy link

just pointing out that the #1876 discussion resulted to actually implement source maps in dev mode.
I'm waiting for this feature for migrating all of my projects to Snowpack. really, this is the most important missing feature.

After investigate, and to my surprise Vite support this very well! I can debug a nwjs and electron app without problem with Vscode., too bad I loved snowpack for several months, but this missing feature was a pain for dev !
image

Maybe look a Vite source code to try understand how to correctly implement a sourcemaps in dev mode, it might help.

@lukejacksonn
Copy link
Contributor

Some work has been done on this recent in #3271 it is an initial attempt but would love to hear if the changes there helped anyone (or hindered 😅)! You should see "out of the box" source maps for any JSX/TSX, svelte files and vue templates in development mode.

@sant123
Copy link

sant123 commented May 20, 2021

Hey @lukejacksonn, thank you for this work. However it only works for a single file.

image

This is the template I'm currently using:

npx create-snowpack-app testing-sourcemaps --template @snowpack/app-template-react-typescript

And the config

image

@limulus
Copy link

limulus commented May 22, 2021

I noticed the same thing as @sant123 in a TypeScript and React project.

Another issue I noticed with the new experimental support is that the sourceMappingURL comment is getting appended to modules coming from node_modules, but there is no corresponding .map file to load, which results in a bunch of 404 error messages for the .map files in the console when you open dev tools.

This is great progress though! I feel like it’s almost there! 😄

@ghry5
Copy link

ghry5 commented May 31, 2021

@sant123 Setting devOptions.hmr = false makes it work with more than index.ts.

@sant123
Copy link

sant123 commented Jun 8, 2021

You are right @ghry5! Going to follow this workaround for now.

Thank you!

@melink14
Copy link

@limulus There's a specific bug about those node_modules based maps here: #3381

You can essentially fix it with sourcemap: 'inline' but that also led to some test failures when enabling coverage for me. 🤷🏾

@Paosder
Copy link

Paosder commented Aug 17, 2021

Hi team, is there any progress about this? I'm really looking forward to it. 😀

@fredDesign
Copy link

fredDesign commented Sep 3, 2021

PLease, help devs , we need the sourcemap ... My team is pushing for Vite and I cant defend snowpack without that feature
(we use "@snowpack/plugin-sass": "^1.4.0" without succes, after many test for configuration)

@jonlepage
Copy link

PLease, help devs , we need the sourcemap ... My team is pushing for Vite and I cant defend snowpack without that feature
(we use "@snowpack/plugin-sass": "^1.4.0" without succes, after many test for configuration)

too late , we all migrate to ViteJs 😉

@AlonMiz
Copy link

AlonMiz commented Sep 4, 2021

we migrated to vite as well

@FredKSchott FredKSchott removed this from Ready to Implement in Triage Apr 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.1 contributors welcome! contributors welcome!
Projects
None yet
Development

No branches or pull requests