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

Embed sourcesContent in source maps #697

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

qnighy
Copy link

@qnighy qnighy commented Nov 2, 2020

This PR changes how source maps and original sources are emitted.

  • Previous: the source map contains "sources": ["application.source.js"]. public/assets/application.source.js will be copied from app/assets/application.js.
  • This PR: the source map contains "sources" ["application.js"], "sourcesContent": ["<the verbatim copy of app/assets/application.js>"].
    • We no longer need to add the artificial .source extension because there's no concern about file name conflicts.

There are two main purposes of source maps: for using in browsers' developer tools, and for error reporting services, like New Relic or Sentry. For the latter purpose, we usually upload source maps directly to the service and delete them before uploading to e.g. S3. Without sourcesContent bundled in the source maps, we won't be able to provide sufficient information to the error reporting service. Moreover, we need to remove application.source.js etc. as well to avoid accidental source code leakage.

Fixes #580.

Question: should we instead provide the embed_source_in_source_maps flag and let users decide the way sources are provided?

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 this pull request may close these issues.

Allow the source map to include the sourcesContent component
1 participant