Skip to content

Fix for sourcemap file locations#481

Closed
chrisnicola wants to merge 1 commit into
babel:masterfrom
chrisnicola:fix-sourcemaps
Closed

Fix for sourcemap file locations#481
chrisnicola wants to merge 1 commit into
babel:masterfrom
chrisnicola:fix-sourcemaps

Conversation

@chrisnicola
Copy link
Copy Markdown

babel-loader was setting options.sourceFileName to a relative path, but this does not seem to work correctly anymore with either webpack or babel itself and causes incorrect source map file data in webpack.

Setting it to an absolute path seems to fix the problem.

Fixes #480 and possibly also #93.

Comment thread package.json
"lib"
"src"
],
"main": "lib/index.js",
Copy link
Copy Markdown
Member

@danez danez Jun 25, 2017

Choose a reason for hiding this comment

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

can you please revert this. lib is correct.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Ah I see, because the build script outputs to /lib. I mainly made this change so I could install from my git branch for testing and didn't really think about it.

That may also explain why it isn't working with Node <8.0. I still have to do some testing of this today, but I'll revert that.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It looks like using a proper build tarball fixed my issues and everything is working again. Thanks for that.

babel-loader was setting `options.sourceFileName` to a relative path,
but this does not seem to work correctly anymore with either webpack or
babel itself and causes incorrect source map file data in webpack.

Setting it to an absolute path seems to fix the problem.

Fixes babel#480 and possibly also babel#93.
Copy link
Copy Markdown
Contributor

@michael-ciniawsky michael-ciniawsky left a comment

Choose a reason for hiding this comment

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

map.sourceRoot should be empty
options.filename === this.resourcePath right ? (map.filename is ignored)
Are there any possible sources? If, they would need to be absolute paths :)

map.sources = map.sources.map((src) => path.resolve(src))

@chrisnicola
Copy link
Copy Markdown
Author

@michael-ciniawsky I'm unclear what you're referring to. Do you mean this shouldn't be assigned to process.cwd()? https://github.com/chrisnicola/babel-loader/blob/38ed9a3db991a3f098d48f1e4ac734dc6d74fc15/src/index.js#L124

@michael-ciniawsky
Copy link
Copy Markdown
Contributor

According to @sokra sourceRoot should be '' and webpack itself takes care of it. Note that I ask for postcss-loader (CSS), so it might vary here, definitely needs triage, but process.cwd() seem to be odd :)

@chrisnicola
Copy link
Copy Markdown
Author

@michael-ciniawsky you are correct that sourceRoot doesn't need to be set. However for the absoluteResourcePath to not simple return the filename the sourceFileName must be set to options.filename

At least that's what my testing showed.

@danez is there anything else I need to do for this PR?

@chrisnicola
Copy link
Copy Markdown
Author

@danez are any other changes required for this.

@michael-ciniawsky I've tested this and it does work with sourceRoot set to '' or unset, but it still requires that the sourceFileName be set to the absolute path of the file. It doesn't seem to matter what the sourceRoot is set to.

@chrisnicola
Copy link
Copy Markdown
Author

@danez hey I'm sure your busy, so I'd be happy to take care of any work necessary to get this merged in. Is there anything I can do?

@chrisnicola
Copy link
Copy Markdown
Author

Still hoping to see this merged sometime. Right now we are packaging the fix manually to make sure source maps work.

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.

Breaks absolute file paths for source map filename templates

3 participants