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

keep path of source file in source map #895

Merged
merged 1 commit into from Aug 3, 2017
Merged

Conversation

Elevista
Copy link

In chrome devtool

Before
before

After
after

Copy link
Member

@kazupon kazupon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you provide this source files repository please?
I want to look the structure.

@Elevista
Copy link
Author

@kazupon sure. https://github.com/Elevista/temp
npm run start will run server with webpack build

@kazupon
Copy link
Member

kazupon commented Aug 3, 2017

@Elevista
Thanks!
It looks good to me.

/ping @yyx990803
Please check it!

@yyx990803 yyx990803 merged commit 1cff850 into vuejs:master Aug 3, 2017
@weizhenye
Copy link

I'm guessing this PR cause a bug about path.

I have followed this tutorial to set up a test workflow with friendly coverage report. With vue-loader@13.0.2,everything works fine as expected, however when I update to vue-loader@13.0.3, the path of .vue files showed in html coverage report is wrong.

Before:

before

before-demo

After:

after

after-demo

It seems the path of .vue files is repeated and gets a wrong result.

Unable to lookup source: F:\test\src\views\src\views\Demo.vue(ENOENT: no such file or directory, open 'F:\test\src\views\src\views\Demo.vue')
Error: Unable to lookup source: F:\test\src\views\src\views\Demo.vue(ENOENT: no such file or directory, open 'F:\test\src\views\src\views\Demo.vue')
    at Context.defaultSourceLookup [as sourceFinder] (F:\test\node_modules\istanbul-lib-report\lib\context.js:15:15)
    at Context.getSource (F:\test\node_modules\istanbul-lib-report\lib\context.js:74:17)
    at Object.annotateSourceCode (F:\test\node_modules\istanbul-reports\lib\html\annotator.js:175:38)
    at HtmlReport.onDetail (F:\test\node_modules\istanbul-reports\lib\html\index.js:217:39)
    at LcovReport.(anonymous function) [as onDetail] (F:\test\node_modules\istanbul-reports\lib\lcov\index.js:24:24)
    at Visitor.(anonymous function) [as onDetail] (F:\test\node_modules\istanbul-lib-report\lib\tree.js:34:30)
    at ReportNode.Node.visit (F:\test\node_modules\istanbul-lib-report\lib\tree.js:123:17)
    at F:\test\node_modules\istanbul-lib-report\lib\tree.js:116:23
    at Array.forEach ()
    at visitChildren (F:\test\node_modules\istanbul-lib-report\lib\tree.js:115:32)
    at ReportNode.Node.visit (F:\test\node_modules\istanbul-lib-report\lib\tree.js:126:5)
    at F:\test\node_modules\istanbul-lib-report\lib\tree.js:116:23
    at Array.forEach ()
    at visitChildren (F:\test\node_modules\istanbul-lib-report\lib\tree.js:115:32)
    at ReportNode.Node.visit (F:\test\node_modules\istanbul-lib-report\lib\tree.js:126:5)
    at Tree.visit (F:\test\node_modules\istanbul-lib-report\lib\tree.js:158:20)

@Elevista
Copy link
Author

Elevista commented Aug 17, 2017

@weizhenye This fix creates virtual path of source file for source map. not the real path. for keeps file path in source map
Could you give me this repo for check?
Maybe I can fix this

@weizhenye
Copy link

@Elevista
I checked every commits between v13.0.2 and v13.0.3, I can be confirm this issue is introduced by this PR.

Reproduce:

  1. Download this gist ZIP.
  2. Move app.js to src/app.js.
  3. Move Demo.vue to src/views/Demo.vue.
  4. Move test.js to test/test.js.
  5. Move Demo.spec.js to test/Demo.spec.js.
  6. npm i
  7. npm test
  8. Open coverage/index.html

@Elevista
Copy link
Author

@weizhenye I exactly got what I missed
image
I'll solve this. thank you

@Elevista
Copy link
Author

@weizhenye
I found istanbul-instrumenter-loader references sourceRoot property of source map
https://github.com/mattlewis92/karma-coverage-istanbul-reporter/blob/e419a634d2f954566cfb1c444f4933947343ca17/src/util.js#L30
I provide it so, the problem solved

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.

None yet

4 participants