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

Proposal: make it possible to support IE11 #4115

Closed
phillipskevin opened this issue Apr 19, 2018 · 7 comments
Closed

Proposal: make it possible to support IE11 #4115

phillipskevin opened this issue Apr 19, 2018 · 7 comments
Labels

Comments

@phillipskevin
Copy link
Collaborator

phillipskevin commented Apr 19, 2018

TLDR: we will

  • create packages for any globals that need to be polyfilled in order to support IE11
  • provide documentation on how to replace these with polyfills without affecting the global environment

This was discussed on a recent live stream (43:17).

Problem

CanJS 4.0 uses several browser features that are not available or not fully functional in IE11. These include:

  • Map
  • WeakMap
  • Set
  • WeakSet
  • Object.assign

This means that in order for CanJS 4.0 to work, you need to load polyfills like core-js or babel-polyfill.

However, doing this will modify these globals for anyone that is using them, not just CanJS. This might cause problems for other libraries that do not expect the polyfill behavior.

Solution

To fix this we should

1. Remove use of non-supported functionality that we don't need

Things like new Set([ 1, 2, 3 ]) are not suppported by IE11 and can be easily replaced by other things.

var set = new Set();
canReflect.addValues([1,2,3]);

Similarly, use of Object.assign(...) can be replaced by can-assign and can-reflect.assignMap.

2. Document what polyfills are required in order for CanJS 4.0 to work in IE11

For people comfortable polyfilling these features in their browser, we should document what polyfills are necessary and give examples of polyfills that provide the features CanJS needs.

3. Make it possible to use CanJS 4.0 in IE 11 without changing the browser's globals.

To do this, we will

3a. Create a module for each unsupported feature

// can-env/weakset.js
module.exports = WeakSet;

3b. Replace uses of the unsupported features with the module

// before
var renderedElements = new WeakSet();
// after
var WeakSet = require("can-env/weakset");
var renderedElements = new WeakSet();

3c. Provide a module with the necessary polyfills

// can-polyfill/weakset.js
module.exports = function(){ ... } //weakset polyfill

3d. Document how to replace can-env with can-polyfill using different module loaders

For example, with steal this would show:

"steal": {
  "map": {
    "can-env/weakset": "can-polyfill/weakset"
  }
}

3e. Run the canjs test suite in IE11

To test that these polyfills work, we would need to add IE11 back to our Sauce Labs setup and modify our test process to run with the polyfills mapped in IE11.

Related issues

@justinbmeyer
Copy link
Contributor

Didn't want polyfills because their app is displayed in Ads. Don't want to mutate the env.

@thomaswilburn
Copy link
Contributor

thomaswilburn commented Sep 7, 2018

Summary of work I've done so far on these issues:

I've been working on the "ie11-compatibility" branch of canjs, as well as branching for individual modules when they need to be fixed. There's a separate test page (ie-test.html) that loads the polyfills for Promise, Object.assign, and WeakSet. It skips four test suites, for various reasons:

  • "can-component/test/component-viewmodel-observe-test" - requires Proxy
  • "can-query-logic/src/types/make-maybe-test" - uses the class keyword
  • "can-view-autorender/test/test"
  • "can-route-pushstate/test/test"

The last two suites run in iframes, and I haven't figured out the best strategy for polyfilling them yet.

The modules that have been branched but not merged are:

Most of these are pretty minimal changes, but I wanted someone who knows the code better than me to take a look. I've run the tests in Firefox after making these changes, and everything passed, which is reassuring.

Once I had everything running, there were 241 failed assertions out of 5067 in IE11. I've managed to pare that down to 196, and it's worth talking about the changes that I made, so that we can avoid those when writing new code as long as IE support is relevant:

  • As noted above, do not chain .set() and .get() on Map or Set objects, and don't call their constructors with an iterable (they have to be populated in a separate step).
  • IE doesn't support Array.from(), so use canReflect.toArray() instead.
  • IE also doesn't implement the Node interface on Document objects, so you can't call contains() or other methods there. Use the element.ownerDocument.documentElement property if you need to test for element attachment.
  • IE is way pickier about properties created with Object.defineProperty() than other browsers are. If you forget to set configurable: true in modern browsers, but you set the same value when you redefine the property, they'll just silently ignore the call. IE, however, throws an error regardless of the value identity. Basically, if you use Object.defineProperty(), make sure you always set it as configurable.
  • The method shorthand is not supported in object literals, and you can't use template strings. These seem like real quality of life issues that I thought Steal would transpile, but that doesn't seem to be the case.
  • Functions do not automatically get name properties in IE. When writing tests, don't assume the function name will be a constant--use a regex to check the warning or message you expect.
  • DocumentFragments in IE don't have firstElementChild, only firstChild. Depending on how you've set the contents of the fragment, you may need to polyfill this, since there may be text nodes or other garbage in the first child slot.

There's also a hilariously terrible bug that I think I've discovered, where IE will not correctly respect enumerable: false for a property if, at any point in the prototype chain, that property is shadowed by an enumerable version. This seems like a bug waiting to happen, but it's subtle, and chasing down any cases where a property is set via higher inheritance is extremely time-consuming.

A lot of the remaining issues look like they probably have shared causes based on the stack traces I'm seeing back in the tests, but I'm not experienced enough with some of the underlying Can modules to tell what is triggering them yet.

@matthewp
Copy link
Contributor

matthewp commented Sep 7, 2018

@thomaswilburn Great work! If tests are passing I say just go ahead and start making pull requests in each repo.

@thomaswilburn
Copy link
Contributor

Will do!

@cherifGsoul
Copy link
Member

cherifGsoul commented Oct 17, 2018

Packages need to be fixed:

Infrastructure

  • can-bind: Object.assign error

  • can-control

  • can-dom-events: new Event() Object doesn't support this action

  • can-event-queue/map: Error on dispatching events

  • can-queues: log tests fail

  • can-reflect-dependencies: test fails, Unable to get property 'whatChangedMe' of null

  • can-view-live

  • can-view-parser PR

  • can-view-scope: Object.assign error

  • can-stache-converters: ✔️ Object.assign error, ✔️ Invalid argument from can-dom-mutate

  • can-attribute-observable: Object.assign (Kevin)

  • can-dom-mutate: ✔️Array.from, ✔️Expected ":" SyntaxError, ✔️Object doesn't support contains, ✔️Object doesn't support this action, ✔️ Invalid argument here(Kevin)

  • can-dom-data-state: Object doesn't support contains (Kevin)

@cherifGsoul
Copy link
Member

cherifGsoul commented Oct 30, 2018

Packages need to be fixed:

Core:

  • can-component (Cherif,Kevin) PR, window.item issue

  • can-define

  • can-route (Cherif) PR

  • can-route-pushstate

  • can-stache ✔️Array.prototype.includes, ✔️Bracket test failure (Symbols) (Kevin)

  • can-stache-bindings (Kevin)

  • can-query-logic

  • can-value

  • can-connect (Cherif) PR

  • can-stache-route-helpers

@cherifGsoul
Copy link
Member

cherifGsoul commented Nov 2, 2018

Packages need to be fixed:

Ecosystem:

  • can-define-backup (Kevin)
  • can-view-autorender all tests are passing but can-dom-mutate throws an error Object doesn't support this action in line

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

No branches or pull requests

6 participants