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

Ensure source map can be resolved from cached code #662

Merged
merged 2 commits into from
Mar 22, 2016

Conversation

novemberborn
Copy link
Member

In kentcdodds/react-ava-workshop#5 nyc is instrumenting the test files. Due to how our caching precompiler works nyc doesn't realize it's instrumenting the transpiled code. It can't find a source map for the transpiled code causing it to crash when generating coverage reports.

Generating coverage reports for test files shouldn't be necessary (and nyc excludes them by default), but it's not something that should crash either.

This PR adds a source map file comment to the cached code. The path is relative to that of the original file, allowing nyc to resolve the source map.

It also changes the source map file extension to the more conventional .js.map.

I think this is a better fix than #655 since we don't have to include the source map in an inline comment. The solution might be too specific to nyc though I imagine other module-loading-intercepting-libraries would behave the same way.

This is the more commonly used extension for source map files.

Fix the test which erroneously looked for `.js` files where it meant `.map`.
When loading the test file, test workers intercept the require call and load the
cached code instead. Libraries like nyc may also be intercepting require calls,
however they won't know that different code was loaded. They may then attempt to
resolve a source map from the original file location.

This commit adds a source map file comment to the cached code. The file path is
relative from the directory of the original file to where the source map is
cached.

Add a test which mimics how nyc resolves the source map.
@mention-bot
Copy link

By analyzing the blame information on this pull request, we identified @spudly, @jokeyrhyme and @sotojuan to be potential reviewers

@jamestalmage
Copy link
Contributor

LGTM

sindresorhus added a commit that referenced this pull request Mar 22, 2016
…mments

Ensure source map can be resolved from cached code
@sindresorhus sindresorhus merged commit 92e034b into master Mar 22, 2016
@sindresorhus sindresorhus deleted the add-test-file-source-map-comments branch March 22, 2016 16:21
@sindresorhus
Copy link
Member

LGTM2

novemberborn added a commit that referenced this pull request Apr 5, 2016
The generateMapFileComment() method used in #662 isn't available in v1.1.2.
sindresorhus pushed a commit that referenced this pull request Apr 5, 2016
* convert-source-map@^1.2.0

The generateMapFileComment() method used in #662 isn't available in v1.1.2.

* use already required convert-source-map dependency

No need to require the generateMapFileComment() method separately.

Also remove unnecessary linebreak at the end of the file.
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