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

Respect babel sourceMaps option #158

Merged
merged 1 commit into from
Jun 25, 2017
Merged

Conversation

dwickern
Copy link
Contributor

The babel.sourceMaps option stopped working since the switch to ember-cli-babel. The option is used to enable ES6 sources in the debugger.

This PR enables sourceMaps under the ember-cli-babel.sourceMaps config option and also babel.sourceMaps for backwards compatibility.

Originally implemented in ember-cli/ember-cli#6458
Fixes (again) ember-cli/ember-cli#6066

index.js Outdated

let options = {
annotation: providedAnnotation || `Babel: ${this._parentName()}`
annotation: addonOptions.annotation || `Babel: ${this._parentName()}`,
sourceMaps: addonOptions.sourceMaps || babelOptions.sourceMaps
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, I'm not sure that we should do this sort of fallback. Each option needs to have a single place to live, if we have to support both locations one should be deprecated.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought babel was deprecated. At least, I remember fixing a deprecation error when I upgraded to ember-cli 2.12

Copy link
Member

Choose a reason for hiding this comment

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

Both are still valid, but each of them for different types of options.

At this point, I would say that sourceMaps should go in babel config (since we do pass it through directly to babel itself), and that we shouldn't allow it to be specified in options['ember-cli-babel'].

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. I won't bother with a warning or deprecation message since sourceMaps was never valid inside ember-cli-babel to begin with

Copy link
Member

Choose a reason for hiding this comment

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

Perfect!

@rwjblue rwjblue merged commit 3f9f223 into emberjs:master Jun 25, 2017
@dwickern dwickern deleted the sourcemaps branch June 25, 2017 18:59
@thefonso
Copy link

thefonso commented Jul 1, 2017

can someone point me to instructions on how to use this?

@dwickern
Copy link
Contributor Author

dwickern commented Jul 1, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants