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

Add support for runtime inline source maps #118

Merged
merged 5 commits into from
Oct 16, 2016

Conversation

kpdecker
Copy link
Contributor

@kpdecker kpdecker commented Dec 3, 2015

Adds the hookRequire option which monkey patches _compile to support require extension hooks that use inline source maps. This is necessary because the default retrieveFile implementations point to the original source file when doing runtime transpilation and thus don’t have the source map attached.

kpdecker added a commit to kpdecker/nyc that referenced this pull request Sep 15, 2016
This allows the user to control the source map output post instrumentation.

Not fully straightforward right now, but when combined with a fork of source-map-support that has inline support (or placing the files in the proper location), end to end covered source maps can be achieved.

For runtime source-map-support, see evanw/node-source-map-support#118. Will be forking to @kpdecker/source-map-support soon, since the canonical project appears to be dead.
@bcoe
Copy link

bcoe commented Oct 2, 2016

@evanw just bumping this issue, would love to add this functionality to IstanbulJS/nyc.

@LinusU
Copy link
Collaborator

LinusU commented Oct 2, 2016

This looks good to me, could you please add some documentation and then we could merge :)

@kpdecker
Copy link
Contributor Author

kpdecker commented Oct 9, 2016

@LinusU added docs and also refactored the installed flag to make it global to avoid conflicts if multiple versions are loaded.

@LinusU
Copy link
Collaborator

LinusU commented Oct 9, 2016

Any idea why Travis is failing?

@kpdecker
Copy link
Contributor Author

kpdecker commented Oct 9, 2016

Looks like it's a slight variation in the text formatting between node versions. I'll make the regex more relaxed.

@kpdecker
Copy link
Contributor Author

kpdecker commented Oct 9, 2016

Actually, it looks like a frame is removed in Node 6.7 (or earlier) which makes the test puke on master as well. Seems like removing the test is the easiest way to handle since maintaining it across multiple Node versions sounds painful.

Adds the hookRequire option which monkey patches _compile to support require extension hooks that use inline source maps. This is necessary because the default retrieveFile implementations point to the original source file when doing runtime transpilation and thus don’t have the source map attached.
This fixes relative url mapping for inline source maps.
This is not consistent across different Node versions.
@kpdecker
Copy link
Contributor Author

kpdecker commented Oct 9, 2016

Rebased and simplified the test to avoid the environment dependent behavior.

@LinusU
Copy link
Collaborator

LinusU commented Oct 16, 2016

Thank you for this!

@LinusU LinusU merged commit 904bf87 into evanw:master Oct 16, 2016
@LinusU
Copy link
Collaborator

LinusU commented Oct 16, 2016

Published as 0.4.4

@kpdecker
Copy link
Contributor Author

Thanks!

@LinusU
Copy link
Collaborator

LinusU commented Oct 18, 2016

@kpdecker require('module') doesn't seem to be available on some runtimes, any ideas on how to fix this? See #153

@kpdecker kpdecker deleted the runtime-inline branch November 6, 2016 21:17
bcoe pushed a commit to istanbuljs/nyc that referenced this pull request Nov 7, 2016
This allows the user to control the source map output post instrumentation.

Not fully straightforward right now, but when combined with a fork of source-map-support that has inline support (or placing the files in the proper location), end to end covered source maps can be achieved.

For runtime source-map-support, see evanw/node-source-map-support#118. Will be forking to @kpdecker/source-map-support soon, since the canonical project appears to be dead.
@cspotcode
Copy link

@kpdecker do you remember (6 years later!) why you opted to Use global hook flag rather than module local?

It sets Module.prototype._compile.__sourceMapSupport as a global. However, the fileContentsCache and sourceMapCache are locals. If two different versions of source-map-support are installed at once, only one of them will get a populated fileContentsCache. Also, if any other libraries wrap Module.prototype._compile in a similar fashion, that flag will go away. I believe there may have been a good reason for doing this, but I don't know what it was.

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.

4 participants