Add support for sourcemap #5
Conversation
|
I'll fix build and make more manual tests tomorrow. |
|
Build fixed, and works on TypeScript and ES6 using #inline-source-map. |
There was a problem hiding this comment.
I think we should consider one more case - when previous source map does not exist even though the user enables source map. (e.g. there are no loader before this loader)
We can use this.sourceMap whether we should output source map.
https://webpack.js.org/api/loaders/#sourcemap
lib/index.js
Outdated
| module.exports = function(contents) { | ||
| const sourceMap = require('source-map') | ||
| const SourceMapConsumer = sourceMap.SourceMapConsumer | ||
| const SourceMapGenerator = sourceMap.SourceMapGenerator |
There was a problem hiding this comment.
Can we remove this line since it's not used?
lib/index.js
Outdated
| @@ -39,8 +45,30 @@ module.exports = function(contents) { | |||
| } | |||
| }) | |||
|
|
|||
| return genCode.generate(res) | |||
| + (hasComponent ? genHotReload(id) : '') | |||
| if (hasComponent) { | |||
There was a problem hiding this comment.
I prefer to negate this condition and return in this block early so that we can reduce the nesting level 🙂
…cratch when previous sourcemap is undefined
|
Should be ok now. Thanks for the really good review ! |
lib/index.js
Outdated
| const id = genId(this.resourcePath) | ||
| let hasComponent = false | ||
|
|
||
| const ast = acorn.parse(contents, { | ||
| sourceType: 'module' | ||
| sourceType: 'module', |
There was a problem hiding this comment.
I also think I can remove locations and sourceFile option when this.sourceMap is falsy.
|
@Toilal Looks great. Can we fix the lint error? 🙂 |
|
Fixed |
|
Thanks. Great work! 👍 |
Close #4
Well, I dive deep into sourcemap generation to understand how to implement this, bit it seems to be the right way to support sourcemap generated by any previous loader in the chain.
Tested with TypeScript.