systemjs / systemjs

Dynamic ES module loader
MIT License
12.94k stars 1.09k forks source link

Injected script execution delay in IE devtools and Firebug #814

Closed guybedford closed 8 years ago

guybedford commented 9 years ago

The script injection approach in https://github.com/systemjs/systemjs/blob/master/lib/global-eval.js#L77 is breaking down when the devtools are open only with a large number of executions in IE11 and Edge in https://github.com/angular/angular/pull/4260.

This breakdown seems to be that there may be some internal limit to how many scripts IE can process at once before it stops executing them with the devtools. It is also worth checking if these executions are always synchronous or if that property is breaking down as well.

It may be possible to resolve the issue through some queueing mechanism for execution, which should be investigated.

On the other hand, we can reduce the number of executions running through this mode by only applying the injected execution mode for global and AMD executions which expect top-level variable assignment to be global assignment.

guybedford commented 9 years ago

Apparently this also happens with Firebug!

guybedford commented 9 years ago

Possible ways forward here:

  1. In a breaking change of SystemJS, just disable support for var globals entirely.
  2. Detect if the devtools are open, and disable inline script-based eval in those cases only
  3. Apply global code transformations in-browser to detect free var globals and scope them as assignments.

(2) and (3) seem like a lot.

It may be worth just going with a hard decision on (1), and documenting this carefully for users.

atsu85 commented 9 years ago

I also stumbled upon this issue with Firebug. For me first option (asking developers not to use firebug) seems a lot to be asked from developers. Sadly it looks like I'm not migrating from RequireJS to SystemJS if this will be the "solution" from SystemJS. As i'm not adding any globals myself (probably they have crept in through some dependency that I haven't figured out) I really hope that You'll find some time for decent solution.

jdanyow commented 8 years ago

I think I might be hitting this issue too. Here's an app that runs in firefox without devtools, then fails to run with devtools:

breaks here

Same app works in all other browser I've tested- chrome, edge, mobile safari, safari.

Another app has an unrelated bug that only impacts firefox but I can't debug it because of this ^^^.

guybedford commented 8 years ago

I'm thinking it may make sense to introduce a special mode here:

System.config({
  globalEvaluationScope: false
});

Such a mode would then use default eval for evaluation, with the caveat that var globalName = 'x' would not write to the global object.

We can then carefully document these evaluation features and note this debugging catch strongly somewhere.

guybedford commented 8 years ago

I've implemented this in https://github.com/systemjs/systemjs/commit/c9ef9e6a7bd5b564c904b6f06f0655b3b9b522f2.

This way System.config({ globalEvaluationScope: false }) will fix this case as well as jsdom execution support.

jdanyow commented 8 years ago

Awesome!

atsu85 commented 8 years ago

Thanks @guybedford! When could we expect the next release of systemjs? I'd like to give it a try :)

guybedford commented 8 years ago

Released in SystemJS 0.19.5.

atsu85 commented 8 years ago

As i have some libs with AMD and other with system, it seems now there is similar problem with system module format:

Unexpected anonymous System.register call. from this line: https://github.com/systemjs/systemjs/blob/master/lib/register.js#L149

capture

guybedford commented 8 years ago

@atsu85 that sounds like a separate issue. If you can perhaps post steps to reproduce that would help to help!

atsu85 commented 8 years ago

Hi @guybedford, unfortunately it looks like proposed solution doesn't work(at least not always) with Firebug, and i still get

TypeError: Unexpected anonymous AMD define

even with SystemJS 0.19.5. To show that proposed fix doesn't work, i've created dedicated branch for this issue: https://github.com/atsu85/skeleton-navigation/tree/systemjs%23814-stillNotResolved

To run the app You need NodeJS and to run few commands on command line - see: https://github.com/atsu85/skeleton-navigation/tree/systemjs%23814-stillNotResolved#running-the-app

guybedford commented 8 years ago

@atsu85 thanks for posting this update. I've fixed this issue in https://github.com/systemjs/builder/commit/d82be245c53c631213c703ae350094d96ee0a76b.

guybedford commented 8 years ago

Argh, wrong issue - still looking into this one!

atsu85 commented 8 years ago

Shame, i already started cheering because of email from the previous message.

@guybedford, any progress on this one? Have You managed to reproduce this issue?

guybedford commented 8 years ago

@atsu85 I finally managed to follow the install instructions here, and the app seems to work fine on that branch without any errors. Did you try to run jspm dl-loader --latest to update SystemJS?

atsu85 commented 8 years ago

Hi @guybedford, i tried it, but it didn't work (even before running it the application was using SystemJS v0.19.5). So i tried reproducing this bug from scratch on two computers (win7 and win8.1, both having FireFox 41.0.2 and Firebug 2.0.13), by running following commands:

git clone git@github.com:atsu85/skeleton-navigation.git && \
cd skeleton-navigation && \
git checkout systemjs#814-stillNotResolved && \
npm install && \
jspm install && \
jspm dl-loader --latest && \
gulp watch

that produced output shown here: win8.1: https://gist.github.com/atsu85/6b88656c791eaa9d5ef2 win7: https://gist.github.com/atsu85/831286aa431f1e0a11f7

including

[19:29:44] Starting 'serve'...
[BS] Local URL: http://localhost:9000
[BS] Serving files from: .
[19:29:45] Finished 'serve' after 1.08 s

and when i opened http://localhost:9000 with Firefox when firebug was disabled, then everything worked as expected on both computers, but when i enabled Firebug on Win 8.1 PC, i saw again this error in my main PC:

Error: Unexpected anonymous AMD define. Evaluating http://localhost:9000/jspm_packages/github/aurelia/pal@0.2.0.js Error loading http://localhost:9000/jspm_packages/github/aurelia/pal@0.2.0.js as "aurelia-pal" from http://localhost:9000/jspm_packages/github/aurelia/bootstrapper@0.18.0/aurelia-bootstrapper.js

And to make sure, that I'm really using current latest version of SystemJS, i verified from firebug network and scripts tag, that systemjs.js file contains expected version:

SystemJS v0.19.5

For some reason the other computer (win7) was using SystemJS v0.18.17 instead of v0.19.5 even after rerunning jspm dl-loader --latest and on that PC (Win7, systemjs 0.18.17) application opened even with enabled Firebug.

So my initial thought is that it looks like this bug is a regression since v0.18.17 and it is still not fixed in v0.19.5 (at least it looks like it is not fixed for Windows 8.1 with FireFox 41.0.2 and enabled Firebug 2.0.13, but i'm not 100% sure).

However, I don't understand why different PC's get different versions of SystemJS?

I'll try to look into this issue again in few hours, but if You have any hints, i'd gladly listen them :)

guybedford commented 8 years ago

Argh, sorry, I guess I should have checked you were adding the System.config({ globalEvaluationScope: false }) configuration here?

atsu85 commented 8 years ago

I didn't quite understand what You meant with Your last comment. Did You suspect that i didn't add System.config({ globalEvaluationScope: false }) as You sugested 10 days ago?

The only change in the branch called "systemjs#814-stillNotResolved" (compared to master) was related to adding globalEvaluationScope: false: https://github.com/atsu85/skeleton-navigation/commit/d7fee2b44c960d6fcad0e03dd0a869df2f82cf0e so this shouldn't be an issue.

Are You saying that i'm doing smth wrong? Did You try to reproduce the issue with enabled Firebug with my sample branch?

guybedford commented 8 years ago

Yes I just wanted to double check that you didn't overlook adding the config. Is IE devtools still not working also? Thanks for the follow-ups, I will take another detailed look at this when I can.

atsu85 commented 8 years ago

It is working with IE(Edge) devtools, but not with Firebug.

atsu85 commented 8 years ago

Update: my Win7 PC got different version of systemjs because jspm was globally installed (different versions on different PC's) and systemjs version depends directly on jspm version.

I found out that SystemJS v0.19.0 works(that comes through jspm 0.16.6), but SystemJS v0.19.1 (that comes with jspm 0.16.7) and SystemJS v0.19.5 don't work.

@guybedford if You really didn't manage to reproduce the issue with enabled Firebug, then what was your globally installed jspm version (and systemjs version used in the application) that failed to reproduce this bug?

guybedford commented 8 years ago

@atsu85 I've posted https://github.com/systemjs/systemjs/issues/878.

atsu85 commented 8 years ago

thanks @guybedford

TsumiNa commented 8 years ago

I use meteor with systemjs, implement System.amdDefine to global environment. when I use Faker.js with <script> injection, I got

Uncaught TypeError: Unexpected anonymous AMD define.
(anonymous function) @ system.src.js:3760
(anonymous function) @ system.src.js:2411
define @ system.src.js:3737
(anonymous function) @ faker.min.js:1
(anonymous function) @ faker.min.js:1

error. If I remove global define, it works.

Have any idea?

guybedford commented 8 years ago

@TsumiNa I tested this out including Faker.js build file with meta configuration scriptLoad: true and setting window.define = System.amdDefine, but everything worked out fine for me. If you're still having the issue please post a full replication in a new issue.

DaSchTour commented 8 years ago

Well, so my odyssey ends here, after several issus about karma, karma-systemjs, jspm and coverage :D After I fixed my last issue with https://github.com/karma-runner/karma-coverage/issues/225 I now got exactly this error, but setting globalEvaluationScope: false doesn't help :( It's really a hard fight to set up systemjs, jspm and karama So does the globalEvaluationScope actually work or do I have to search for another solution?