Skip to content

Source maps?#16

Open
petehunt wants to merge 1 commit into
masterfrom
sourcemaps
Open

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

Conversation

@petehunt

Copy link
Copy Markdown
Owner

@sokra is this good?

@gaearon

gaearon commented Jul 15, 2014

Copy link
Copy Markdown
Contributor

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

gaearon commented Jul 15, 2014

Copy link
Copy Markdown
Contributor

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

@sokra

sokra commented Jul 15, 2014

Copy link
Copy Markdown

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

sokra commented Jul 15, 2014

Copy link
Copy Markdown

only with the source-map-loader...

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

@jRiest

jRiest commented Aug 22, 2014

Copy link
Copy Markdown

@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

sokra commented Aug 22, 2014

Copy link
Copy Markdown

@jRiest

jRiest commented Aug 26, 2014

Copy link
Copy Markdown

@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