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

//@sourceMappingURL is getting replaced with //#sourceMappingURL #3

Closed
domenic opened this issue Jun 10, 2013 · 18 comments
Closed

//@sourceMappingURL is getting replaced with //#sourceMappingURL #3

domenic opened this issue Jun 10, 2013 · 18 comments

Comments

@domenic
Copy link

domenic commented Jun 10, 2013

Didn't want to file this with all your source mapping related repos, and it's probably too early to make the switch, but thought it'd be worth tracking.

@thlorenz
Copy link
Owner

Yeah, good idea.
Since all these browsers/engines are auto upgrading, we won't have to worry about backwards compat?

Until then with this mold-source-map transform you could switch this out today if you wanted.

@domenic
Copy link
Author

domenic commented Jun 10, 2013

Since all these browsers/engines are auto upgrading, we won't have to worry about backwards compat?

Yeah agreed. Maybe just let it stay as-is for now and people running nightlies or whatever can use that transform, then in 12 weeks or so when those changesets hit stable you can flip the switch and convert everything from @ to #.

@domenic
Copy link
Author

domenic commented Jul 8, 2013

Chrome is removing the deprecation warning for at least a while. https://code.google.com/p/chromium/issues/detail?id=256636

@thlorenz
Copy link
Owner

thlorenz commented Jul 8, 2013

so any more info on when it would be reasonable to switch to the updated api?

@domenic
Copy link
Author

domenic commented Jul 8, 2013

Chrome 29 I believe, which should be 12 weeks after Chrome 27 which was released on 2013-06-18. Still some time away.

@domenic
Copy link
Author

domenic commented Oct 6, 2013

Chrome 30 is now stable as of October 3; it's probably time to switch.

@thlorenz
Copy link
Owner

thlorenz commented Oct 6, 2013

How is it looking for other browsers that support source maps? Did Firefox make the change.

It's nice to have this, but also not urgent IMHO. I looked into chrome's source a bit and they support both versions right now and I don't see that they'll take that out any time soon.
So it doesn't hurt to wait, but could break if browsers still use older standard.

However if the risk of breaking anything does no longer apply at all, we should move to the new version.
This will affect multiple modules though and we should plan a bit on how to properly do this.

An option that I see is that combine-source-map can process both versions, but always produces the newer version.

@Anachron
Copy link

Anachron commented Oct 7, 2013

Here is the actual status of new SourceMapSyntax support:
http://updates.html5rocks.com/2013/06/sourceMappingURL-and-sourceURL-syntax-changed

I think it's time to get implemented.

@thlorenz
Copy link
Owner

thlorenz commented Oct 8, 2013

@Anachron thanks and agreed.

Now we just need to make sure we do this properly without breaking transpilers that generate sourcemaps in the old style and/or dependent modules.

Changes will actually have to occur in other modules, i.e. we'll generate a # style source url here and make the regex here match # style source urls.
Matching both @ and # style source maps here will ensure that consuming @ style generated source maps (by older transpilers) keeps working.

Also the inline-source-map module has to change here
I'll publish these changes as a major upgrade on these and we'll need to help peeps to upgrade their modules and/or warn the ones that could potentially break due to changes.

There are lots of modules that will have to get upgraded.

I'm starting to compile a list of modules we should PR on after the changes are made and hope to get some of your help on this. Also please make me aware of modules that may break due to these changes so we can at least let the authors know about this.

@Anachron
Copy link

Anachron commented Oct 8, 2013

I took a quick look for every dependency:

https://github.com/dstrek/node-browserify-express - Nothing to do here
https://github.com/sidorares/browserify-jade - Nothing to do here
https://github.com/thlorenz/caching-coffeeify - Nothing to do here
https://github.com/gromnitsky/coffee-inline-map - Nothing to do here
https://github.com/uber/coffee-middleware - Nothing to do here
https://github.com/jnordberg/coffeeify - Nothing to do here
https://github.com/thlorenz/coffeeify-redux - Nothing to do here
https://github.com/AndrewGaspar/embed-source-map - Already supports new sourcemaps (https://github.com/AndrewGaspar/embed-source-map/blob/master/embed-source-map.ts#L68)
https://github.com/sidorares/embed-source-map - Add new syntax (https://github.com/sidorares/embed-source-map/blob/master/index.js#L23-L24)
https://github.com/maxtaco/icsify - Nothing to do here
https://github.com/thlorenz/mold-source-map - Nothing to do here
https://github.com/mdlawson/piping-browser - Nothing to do here
https://github.com/featurist/pogoify - Nothing to do here
https://github.com/bodil/typeify - Add new syntax (https://github.com/bodil/typeify/blob/master/index.js#L32)
https://github.com/johnkpaul/requireify - Nothing to do here

Nothing to do here: Just force tests on new syntax.

I checked the sourcecode and looked for places where sourcemaps are used/manipulated without using combine-source-map-api. Please recheck, maybe I missed something.

@thlorenz
Copy link
Owner

thlorenz commented Oct 8, 2013

@Anachron I'm mainly talking about bumping the versions of modules that we'll update, i.e. here.

Additionally if there are tests, we need to ensure that they still pass and/or update those as well.

@Anachron
Copy link

Anachron commented Oct 8, 2013

@thlorenz Oh, but I even pointed out what should be different in those modules to keep the compatibility :)
So I guess I did more than expected ...

Anyway, I will check if I can do some PRs, I never did any before, tbh.

@thlorenz
Copy link
Owner

thlorenz commented Oct 8, 2013

@Anachron don't worry about it - at least until I actually upgraded convert-source-map and inline-source-map.
I'll publish those with new major version and then we can start upgrading the modules that depend on them by bumping the versions in the package.json files.

@Anachron
Copy link

Anachron commented Oct 9, 2013

@thlorenz When will you submit these upgrades? Will you create a new version (tag), create a branch or keep changes pending?

@thlorenz
Copy link
Owner

thlorenz commented Oct 9, 2013

I'm planning to update convert-source-map and inline-source-map within the next few weeks (none of this is urgent) and will update this thread once that is done.

@Anachron
Copy link

Hey @thlorenz, how is it going?
Firefox already throws the following warning:
[15:09:38.747] SyntaxError: Using //@ to indicate source map URL pragmas is deprecated. Use //# instead @ https://subdomain.example.com/javascripts/main.js:13

@thlorenz
Copy link
Owner

thlorenz commented Nov 5, 2013

The following changes have been made to conform to new source map specs:

As a result, these modules consume @ sourceMappingURLs and # sourceMappingURLs to stay backwards compat, however they generate source maps that conform to the new spec, i.e. //# sourceMappingURL=.

This will keep most dependent modules working, except the ones that try to consume the produced sourcemaps from either of these modules.

These dependents need to fix the incompatibility when they upgrade to 0.3 of either of these modules.

The next step is to update browser-pack to use these latest versions in order to make browserify sourcemaps conform to the new spec.

I checked through the above mentioned modules and it seems that none of them will have compatibility problems except typefy which needs to change this line.
I checked off all the others.

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

No branches or pull requests

3 participants