-
Notifications
You must be signed in to change notification settings - Fork 63
fix(index): correct loader sourcemaps usage #67
Conversation
a6f974b
to
c4d1438
Compare
Codecov Report
@@ Coverage Diff @@
## master #67 +/- ##
========================================
- Coverage 100% 90.9% -9.1%
========================================
Files 2 2
Lines 6 11 +5
Branches 0 2 +2
========================================
+ Hits 6 10 +4
- Misses 0 1 +1
Continue to review full report at Codecov.
|
appveyor.yml
Outdated
@@ -19,6 +19,7 @@ matrix: | |||
fast_finish: true | |||
install: | |||
- ps: Install-Product node $env:nodejs_version x64 | |||
- cmd: npm i -g npm@latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this fixing the current falky builds for branches
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's to fix appveyor on node 4, which uses npm 2.x which apparently isn't compatible with babel
test/utils/loader.js
Outdated
import path from 'path'; | ||
import loader from '../../src/cjs'; | ||
|
||
const windowsToUnixPathSeparators = str => str.replace(/\\/g, '/'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const normalize = str => str.split(path.sep).join('/')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
path.sep :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done 👍
c4d1438
to
ef94627
Compare
@mattlewis92 - Just landed #66 I'll take a look at appveyor and figure out why it's not reporting. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Appveyor should be sorted, needs a quick rebase & g2g
@d3viant0ne done! |
Thanks @d3viant0ne 😄 Gave the RC a spin in our app and its all working smoothly now. We probably want to land #65 before cutting 3.0.0 final, as that PR is breaking. |
I was wrong about the sourcemap behaviour, turns out webpack does actually pass the sourcemap as the second arg to loaders, this PR restores the previous behaviour (with a perf boost by not trying to parse for an inline sourcemap if there is already a source map available)
Depends on #66