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

ie8 support #1517

Closed
opsb opened this issue Jul 29, 2014 · 31 comments
Closed

ie8 support #1517

opsb opened this issue Jul 29, 2014 · 31 comments

Comments

@opsb
Copy link

opsb commented Jul 29, 2014

I've put together a demo app that uses es5-shim to add the missing bits. https://github.com/opsb/ember-cli-ie8-demo. The Brocfile loads up the shim and sham but apart from that the repo is a vanilla ember-cli app(they're loaded before ember via the vendor-files option). If I just load up the shim I get an error when Object.create is used. The sham polyfills this but unfortunately when it's included in the build it also causes ie to crash completely.

@opsb
Copy link
Author

opsb commented Jul 29, 2014

I managed to get it working with a hacked up version of the es5-sham. I haven't pinned down what caused the error yet, I started with an empty file and kept adding until the app started working (fortunately this happened before IE started crashing). I've also added it to a forked version of ember-todos-cli for a slightly more thorough test.

The app I'm working on still isn't working though(it's fine in IE9) so I'll have to keep digging tomorrow.

@opsb
Copy link
Author

opsb commented Jul 29, 2014

My main app worked fine after reinstalling all the node modules so that version of the es5-sham looks like it works fine. It could do with a little tidying up around the edges but for now I'm just happy to have it running in ie8!

@opsb
Copy link
Author

opsb commented Jul 29, 2014

I've pinned it down to a single line and updated the gist to include everything except the problematic line https://gist.github.com/opsb/1d2569b89ab7d9d0f662#file-es5-sham-js-L345. It gets called once when ember starts up. IE8's debugger crashes when I try to open it so it's a bit hard to pin it down further than that.

@stefanpenner
Copy link
Contributor

sorry for the silence on this thread, i am reading it. Just extremely busy with work this week.

This sounds like an issue that should also be opened up on es5-shim/sham repo

@opsb
Copy link
Author

opsb commented Jul 29, 2014

@stefanpenner yup, it does seem to be a problem with their implementation, the values that were causing ie8 to crash were as below: (quite remarkable really that this could cause ie8 to crash...)

object = {a:1}
descriptor = {value:2}
object[property] = descriptor.value;

I've opened an issue at es-shims/es5-shim#264

@stefanpenner
Copy link
Contributor

@opsb nice thanks for digging into this.

@opsb
Copy link
Author

opsb commented Jul 31, 2014

@stefanpenner see the thread over at es-shims/es5-shim#264 for more details, but the short version is that ember stubs Object.create itself and when it does so it sets a flag isSimulated which is used in other parts of the ember codebase. Because es5-sham stubs Object.create before ember the flag is never set. There's an Ember.ENV.STUB_OBJECT_CREATE which you can use to force ember to stub Object.create as well and thus set the flag.

Where would be the place to put Ember.ENV.STUB_OBJECT_CREATE = true ? I'm thinking index.html is probably the right place to do this as I can put an ie8 conditional around it. Do you think this is the best approach?

@opsb
Copy link
Author

opsb commented Jul 31, 2014

The ember-cli-ie8-demo now runs without crashing in ie8. I'll leave the repo up and I've added some instructions for setting up a project to work in ie8.

@ljharb
Copy link

ljharb commented Jul 31, 2014

Why is ember relying on a manually set flag, rather than a feature test? That seems the source of the problem here. If ember did try { Object.create(null) } catch (e) { /* set the flag to true */ } that would more reliably detect if you have a native Object.create or not, for example.

@stefanpenner
Copy link
Contributor

@opsb
Copy link
Author

opsb commented Jul 31, 2014

@stefanpenner I think the issue is the use of the isSimulated flag. It's only set if Object.create hasn't already been stubbed before ember tests for it.

@stefanpenner
Copy link
Contributor

@opsb im not sure how the isSimulated flag could possible change the outcome of: if (!(Object.create && !Object.create(null).hasOwnProperty))

@opsb
Copy link
Author

opsb commented Jul 31, 2014

@stefanpenner it won't, but the flag not being set (which is the case if you use es5-sham before ember - neccessary with ember-cli) will mean the following workaround won't be applied.

https://github.com/emberjs/ember.js/blob/v1.6.1/packages_es6/ember-runtime/lib/keys.js#L15

@stefanpenner
Copy link
Contributor

@opsb my comment above was referring to ember canary, not 1.6

@opsb
Copy link
Author

opsb commented Jul 31, 2014

@stefanpenner I finally got the debugger working so I can see what's going on a bit more. The call stack is pretty deep where the error gets thrown, 2904 calls, most of it recursion inside the wireState method from ember-data. The call originates at RootState = wireState(RootState, null, "root"); I'm not familiar with the code so I'm not sure whether this is a reasonable level of recursion or if there's an infinite loop happening.

@stefanpenner
Copy link
Contributor

@opsb thats seems absolutely crazy. Do you mind providing steps for me to reproduce i'll try to find some time to dig in.

@stefanpenner
Copy link
Contributor

@opsb should we move this over to ember.js proper or does this not happen with master?

@opsb
Copy link
Author

opsb commented Jul 31, 2014

@stefanpenner I'm not sure where to put this issue as it occurs in ember-data but I presume it's just the first piece of code to hit the issue caused by the implementation of Object.create. To recreate just check out https://github.com/opsb/ember-cli-ie8-demo, change the ember version to canary and then load it up in ie8 (I use the image from modern.ie). To get it to break at the right point I use a call counter inside Object.create i.e.

var callCounter = 0;
create = Object.create = function create(prototype, properties) {
    callCounter++;
    if(callCounter === 3170){
      throw "finished";
    }
    ....
}

@jdjkelly
Copy link
Contributor

jdjkelly commented Aug 1, 2014

Just ran into this too @opsb - thanks for digging in

@jdjkelly
Copy link
Contributor

jdjkelly commented Aug 1, 2014

This is also happening on the jj-abrams resolver: https://github.com/stefanpenner/ember-jj-abrams-resolver/blob/master/dist/ember-resolver.js#L47

IE8 fails here, complaining Object does not support this property or method

@stefanpenner
Copy link
Contributor

@jdjkelly i should have done. typeof Object.create(null) === 'undefined'

@jdjkelly
Copy link
Contributor

jdjkelly commented Aug 1, 2014

PR up here: ember-cli/ember-resolver#63 - matches what Ember canary is doing - I'll submit a version bump PR for ecli if that gets merged

@stefanpenner
Copy link
Contributor

@jdjkelly merged

@jamesreggio
Copy link

Unfortunately, this issue still exists for me on Ember canary (as of emberjs/ember.js@793dbf3).

To be precise, with the basic "Hello Ember.js" app:

  • if I import es5-shim and es5-sham before Ember, IE8 will crash.

  • if I import only es5-shim before Ember, the app will fail to initialize with this error:

    Error while processing route: index, 'Node' is undefined
    

I'm not sure if the latter error is something else entirely, and the original Object.create issue is resolved. Any ideas?

@stefanpenner
Copy link
Contributor

That node error is unrelated and a new regression since Sunday night...

@stefanpenner
Copy link
Contributor

Also entirely unrelated to ember cli

@trabus
Copy link
Contributor

trabus commented Dec 2, 2014

I've been working on trying to get our app working with IE8, and tried importing the ES5 shim and sham in the vendorFiles in my Brocfile. Unfortunately, they are merged in at the end of the vendor.js file, which is causing them to not work at all. Should we be bringing in the user defined vendorFiles before the ember-cli defined vendorFiles instead? Or should we define something like a preVendorFiles that will fulfill this need instead?

If this is desired functionality, I'll create a new issue and work on a PR for this.

@stefanpenner
Copy link
Contributor

@trabus which version of ember are you using?

@trabus
Copy link
Contributor

trabus commented Dec 2, 2014

Also, I finally got IE8 loading with:
ember-cli#0.1.3 (modified to put user defined vendorFiles at the beginning of the vendor.js)
ember-data#1.0.0-beta.12
ember#1.8.1
es5-shim#4.0.5

Removing es5-sham.js entirely fixed the crashing issue described in ember data #9430 (I was still experiencing the same crash even with the 'Ember.create' fix in place).

@MichaelVdheeren
Copy link

@trabus How did you accomplish getting the vendorFiles being put first? I need the es5-shim but not the sham.

@trabus
Copy link
Contributor

trabus commented Dec 12, 2014

I was using Ember CLI 0.1.1, there was a change in 0.1.2 that changed the order of vendorFiles. I made a pull request that fixes this recently. It just got merged into master and will be available in 0.1.5. You will be able to import the file with prepend: true and it will be prepended to the vendor.js.

// Brocfile.js
app.import('bower_components/es5-shim/es5-shim.js', { type: 'vendor', prepend: true });

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

No branches or pull requests

8 participants