Inline source maps as comments #123

Merged
merged 1 commit into from Dec 23, 2013

Conversation

Projects
None yet
4 participants
Contributor

andreypopp commented Dec 3, 2013

I propose inlining source maps as a base64 encoded data uris — that would provide excellent dev experience and doesn't require to serve source maps to browser separately.

@lydell lydell referenced this pull request Dec 3, 2013

Closed

Source Map support #109

Contributor

andreypopp commented Dec 3, 2013

This depends on unreleased yet css@1.5.0 — see visionmedia/css#8

Contributor

kristoferjoseph commented Dec 5, 2013

@visionmedia any objections to having source maps inline?

Contributor

jonathanong commented Dec 5, 2013

@visionmedia can you bump and push css to npm so we can see this PR's status? also, there are two new features in master (i semi-populated the change log already). would be nice to get this in and push a new rework release!

Contributor

kristoferjoseph commented Dec 5, 2013

+1

@ghost ghost assigned tj Dec 6, 2013

Contributor

jonathanong commented Dec 23, 2013

oh my god i have npm rights. i'm going to do a release without this stuff, then tweet @visionmedia about this PR and a css release

Owner

tj commented Dec 23, 2013

i still think inline should be optional, default or not is another thing

Contributor

jonathanong commented Dec 23, 2013

man you've been giving me npm rights on jonathanong in npm. i didn't even know i had that account. now i have to juggle two npm accounts haha

Contributor

jonathanong commented Dec 23, 2013

@andreypopp okay can you rebase and make appropriate changes? i'll merge and release ASAP

Contributor

kristoferjoseph commented Dec 23, 2013

Nice! Inline optional isn't a show stopper for me. As long as sourcemaps work with a flag so I can generate a debug and release version I am happy with however they are implemented. I would always be releasing two versions anyways.

Owner

tj commented Dec 23, 2013

@jonathanong oh haha that's the one you listed on your npm org rights thing hahahaha NPM FTW

inline by default is probably a pretty sane thing, at least then it works out of the box

Contributor

andreypopp commented Dec 23, 2013

Message from commit:

  • rework(src, options) now accepts options arguments which is passed to
    css-parse, we set options.position = true so we can generate sourcemaps later
  • Rework.prototype.toString(options) accepts sourcemap and sourcemapAsObject
    boolean options. The first one instructs css-stringify to generate sourcemap
    and then inlines it in source as base64 encoded data URI. If sourcemapAsObject
    is true then result from css-stringify returned unmodified and thus no
    sourcemap is inlined.
Contributor

andreypopp commented Dec 23, 2013

sourcemapAsObject looks silly, we can rename it

Inline source maps as comments
* rework(src, options) now accepts options arguments which is passed to
  css-parse, we set options.position = true so we can generate sourcemaps later

* Rework.prototype.toString(options) accepts sourcemap and sourcemapAsObject
  boolean options. The first one instructs css-stringify to generate sourcemap
  and then inlines it in source as base64 encoded data URI. If sourcemapAsObject
  is true then result from css-stringify returned unmodified and thus no
  sourcemap is inlined.
Contributor

jonathanong commented Dec 23, 2013

okay merging and releasing. convert-source-map doesn't have component support so i guess it makes sense to be opt-in.

jonathanong added a commit that referenced this pull request Dec 23, 2013

@jonathanong jonathanong merged commit 0dc85e7 into reworkcss:master Dec 23, 2013

1 check passed

default The Travis CI build passed
Details

@andreypopp andreypopp deleted the andreypopp:source-maps branch Dec 23, 2013

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