Skip to content

Source maps?#16

Open
petehunt wants to merge 1 commit intomasterfrom
sourcemaps
Open

Source maps?#16
petehunt wants to merge 1 commit intomasterfrom
sourcemaps

Conversation

@petehunt
Copy link
Copy Markdown
Owner

@sokra is this good?

@gaearon
Copy link
Copy Markdown
Contributor

gaearon commented Jul 15, 2014

BTW it's potentially breaking right? 0.11 is still RC?
Or did JSX not change?

@petehunt
Copy link
Copy Markdown
Owner Author

yes but i bumped jsx-loader version accordingly

@petehunt
Copy link
Copy Markdown
Owner Author

(i doubt anything actually breaks)

@gaearon
Copy link
Copy Markdown
Contributor

gaearon commented Jul 15, 2014

Ah right I didn't notice -rc on the loader version.

@sokra
Copy link
Copy Markdown

sokra commented Jul 15, 2014

hmm.. i don't think it's good...

It need to be something like:

var result = ....transformWithDetails(...);
this.callback(null, result.code, result.map);

@petehunt
Copy link
Copy Markdown
Owner Author

@sokra are inline sourcemaps supported/

@sokra
Copy link
Copy Markdown

sokra commented Jul 15, 2014

only with the source-map-loader...

So it's better to return it directly... also better for performance...

@jRiest
Copy link
Copy Markdown

jRiest commented Aug 22, 2014

@petehunt I added source-map support in my fork here, but it just uses convert-source-map to extract the inlined source map, which I'm sure is far from optimal. Do you want me to create a pull request or do you have a better alternative in the works?

@petehunt
Copy link
Copy Markdown
Owner Author

I'm not really actively working on this so send a pr if it works!

@sokra
Copy link
Copy Markdown

sokra commented Aug 22, 2014

@jRiest
Copy link
Copy Markdown

jRiest commented Aug 26, 2014

@sokra that's much better. Thanks for pointing that out. I re-implemented sourcemaps using that method and submitted a pull request against this branch (sourcemaps) here

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.

4 participants