Update sourcemap implementation

[jRiest]

Implementing sourcemaps according to the feedback from @sokra. I also merged in the latest changes from master.

[jbaiter]

👍 Works fine here :-)

[gaearon]

Does source map need to be opt-in via flag? @sokra, is this the recommended way?

If so, I'd appreciate the docs explaining how to combine flags (I presume ?harmony&sourceMap should work but I'm not sure).

[sokra]

I doesn't need to be opt-in... If you don't use a source-map devtool it's ignored...

The other loaders always emit source-maps...

[jRiest]

No problem. I changed it to always emit them. @sokra perhaps we could have some sort of flag at the loader level which would allow loaders to skip source map generation if it is not necessary.

[sokra]

yup... I'll add one...

[petehunt]

If they always emit source maps it could contribute to increased build time, no?

Sent from my iPhone

On Aug 27, 2014, at 7:56 AM, "Tobias Koppers" <notifications@github.commailto:notifications@github.com> wrote:

yup... I'll add one...

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/19#issuecomment-53584642.

[sokra]

this.sourceMap

[sokra]

@petehunt yes... we may need to patch the loaders to use the new flag...

[jRiest]

@sokra @petehunt I updated it to read the new flag. However, as it stands now, source maps won't be enabled for anyone using webpack < 1.4.0-beta1. Is this the preferred way to handle it, or would it be better to do something like this, where we read the flag if it's there, otherwise we automatically emit source maps?

var result = reactTools.transformWithDetails(source, {
  harmony: query.harmony,
  sourceMap: typeof this.sourceMap === 'boolean' ? this.sourceMap : true
});

Since this loader has not had source map support before I could go either way, but for loaders that currently always emit source maps, I would assume they would take the above approach so as to not break for people who don't update their webpack version.

[gaearon]

@sokra @petehunt

What do you think? Ideally I'd like to wait for this before merging gaearon/react-hot-loader#23.

[sokra]

Looks good 😄

[gaearon]

@petehunt If nothing is blocking, can you bring this in?