Skip to content
This repository was archived by the owner on Jul 13, 2020. It is now read-only.

Add //# sourceURL along with source map. #256

Merged
merged 1 commit into from
Dec 12, 2014

Conversation

dtinth
Copy link
Contributor

@dtinth dtinth commented Dec 4, 2014

For some odd reason, Google Chrome requires sourceURL to be able to
display source maps in stack traces correctly.

For some odd reason, Google Chrome requires sourceURL to be able to
display source maps in stack traces correctly.
@dtinth
Copy link
Contributor Author

dtinth commented Dec 4, 2014

For this test page, if the generated ES5 code from ES6 does not contain the sourceURL directive, it shows up in DevTools like this:

screen shot 2014-12-04 at 12 27 50

With the sourceURL directive, however, it now looks like this:

screen shot 2014-12-04 at 12 28 14

@guybedford
Copy link
Member

Thanks so much for this - I'll try it out soon.

guybedford added a commit that referenced this pull request Dec 12, 2014
Add //# sourceURL along with source map.
@guybedford guybedford merged commit ccc38c8 into ModuleLoader:master Dec 12, 2014
@guybedford
Copy link
Member

Thanks so much for this - that makes a big difference to debugging!

@dtinth
Copy link
Contributor Author

dtinth commented Dec 12, 2014

You're welcome. Thanks for creating this project. I've always wanted to write my apps in ES6.

Even though it works well in Chrome, debugging in Safari is still painful... Somehow, its DevTools does not support source map or source URL.

What I've tried so far is to—instead of evaluating the code using eval()—to put the transpiled script into a Blob, turn it into a URL, and put that into a script tag. Now, whenever something goes wrong, we get actual stack trace, which can be inspected using the DevTool. Even though we are debugging the transpiled code, it's better than having no access to the code at all.

The code to evaluate the script using Blobs look like this:

function __scriptEval(code) {
  return new Promise(function(resolve, reject) {
    var url = URL.createObjectURL(new Blob([code], { type: 'text/javascript' }));
    var script = document.createElement('script')
    script.src = url
    script.onload = function() { resolve() }
    script.onerror = function(e) { reject(e) }
    document.body.appendChild(script)
  })
}

Here's the patch, an incomplete version. Note that it's done against the compiled version of es6-module-loader.

I have some difficulty with it: with a moderately-sized dependency tree, there are some race conditions that caused System.register to be called twice when it shouldn't. It sometimes work, but most of the time it doesn't.

I already used an evaluation queue to make sure that only single __eval is called at any given time, but somehow it didn't work. System.register may have been called from somewhere outside __eval. Could you give me some pointer?

@guybedford
Copy link
Member

We actually do something similar to this in SystemJS already here - https://github.com/systemjs/systemjs/blob/master/lib/polyfill-wrapper-end.js#L42.

I'd also be interested to hear if the process of creating the blob URL affects the performance. It could certainly be an option here since the direct ES6 evaluation feature is only for development in modern browsers.

For the race condition - perhaps we can avoid wrapping the code in a promise and assume execution will be synchronous on appendChild anyway? This is because we need a synchronous lock of the global System.register for the execution to work. I think that's what I'm doing in the SystemJS version anyway?

@guybedford
Copy link
Member

Alternatively if execution that way must be asynchronous, we could do the System.register global manipulation within the evaluation itself.

@matthewp
Copy link
Contributor

I like the Blob idea. There's similar problems with Firefox's debug tools with eval() scripts. Except it's much worse; they flat out don't work. There has been some recent work to fix this on their end but I have still not been able to ever debug es6ml-based code in Firefox which saddens me greatly.

@matthewp
Copy link
Contributor

@guybedford I know you're probably going to say no to this (and i don't blame you)... but if doEval were exposed so that it could be overwritten that would be very nice.

@dtinth
Copy link
Contributor Author

dtinth commented Dec 12, 2014

What I do right now is to just modify es6-module-loader and SystemJS and use my own version instead. That refers to my using the version in the PR before it was merged.

Another alternative is to be able to be able switch between eval strategy through, say, a use of window property or System property. Something like

System.EVAL_STRATEGY = 'eval'   // uses eval() to evaluate script — should be fastest
System.EVAL_STRATEGY = 'script' // injects a script tag with inline code
System.EVAL_STRATEGY = 'blob'   // injects a script tag pointing to a blob

Promises also clutter the stack trace in Chrome, given that a native Promise already exists. Another suggestion is to not enable the Promise polyfill. Currently, I check for Promise to selectively load es6-module-loader-sans-promises instead.

@guybedford
Copy link
Member

@dtinth we polyfill promises always because the included promise implementation is much faster than Chromes native promises (see https://github.com/petkaantonov/bluebird/blob/master/benchmark/stats/latest.md).

@matthewp I actually agree it would make sense to open up the evaluation implementation at this point.

@guybedford guybedford mentioned this pull request Dec 17, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants