acdlite / flummox

Minimal, isomorphic Flux.
http://acdlite.github.io/flummox
1.69k stars 114 forks source link

FluxComponent does not work on IE10 #271

Closed Pavek closed 8 years ago

Pavek commented 8 years ago

I researched the issue and it boils down to babel code implementing inheritance, which doesn't work on IE < 11. Here's the relevant links: http://ricostacruz.com/til/babel-ie-class-inheritance.html https://phabricator.babeljs.io/T3041

Note: it reportedly can be fixed by using some babel build options, but we have another issue here: Flummox npm module comes with already transplied code. I'm not sure why so, and it seems generally incorrect. Having regular source files, users of npm module would have a choice of a) how (for which browsers) to transply the code or b) use bundles in /dist/ that are officially tested ones.

Not sure why and if this is relevant, but bundles in '/dist/' are missed now completely (they were there before 3.6.3).

voronianski commented 8 years ago

@Pavek

Note: it reportedly can be fixed by using some babel build options

Please share the options that can fix that babel behavior.

Not sure why and if this is relevant, but bundles in '/dist/' are missed now completely

dist folder is available now in 3.6.5, it was missed accidentally.

but we have another issue here: Flummox npm module comes with already transplied code. I'm not sure why so, and it seems generally incorrect.

It's a general rule for npm modules to be shipped already transplied.

Pavek commented 8 years ago

Please share the options that can fix that babel behavior.

Please see this comment: https://phabricator.babeljs.io/T3041#66105

voronianski commented 8 years ago

@Pavek

that's strange that Flummox has this bug, as we're already using "loose" es5 preset - https://github.com/acdlite/flummox/blob/master/.babelrc#L3

Pavek commented 8 years ago

If that might help, here's the root cause as we debugged down: In transplied FluxComponent.js:

function _inherits(subClass, superClass) {
...
 if (superClass) Object.setPrototypeOf ? 
      Object.setPrototypeOf(subClass, superClass) :
      subClass.__proto__ = superClass; }

}

IE10 do not support both setPrototypeOf and __proto__ thus _inherits() basically do not deliver what is expected to. I'm not aware of the correct way to fix this with Babel. One option was listed on first link above -- protoToAssign -- but I failed to find a documentation for it because old docs of Babel are gone :(

YuriSizov commented 8 years ago

@voronianski That's not the only problem with the new version of Babel.

There's also a problem with IE8 (yeah, I know...) due to new Babel behavior. Preset es2015 does not include ES3 transformations, which replace unsafe usage of reserved keywords like default.

You need to have

        "plugins": [
          "transform-es3-member-expression-literals",
          "transform-es3-property-literals",
          ...
        ]

to fix that problem.

I would really appreciate if you added those and made Flummox 3.6 IE8-compatible.

Pavek commented 8 years ago

@pycbouh probably it's better to open a new bug for this to make it more track-able.

voronianski commented 8 years ago

@Pavek @pycbouh oh babel.. 5th version was so good :cry:

voronianski commented 8 years ago

I can take a closer look and experiment with plugins, presets and options a bit later, however I cannot test it 'cause I don't have IE.. maybe you have time to play with it right now? @Pavek @pycbouh

YuriSizov commented 8 years ago

@Pavek Ideally, it has to be a bug about Babel configuration, I think. Well, created #272 anyway.

@voronianski Yeah, this change was massive. We just moved to Babel 6, it was very tricky.

YuriSizov commented 8 years ago

@voronianski Sure.

I can say right now, though, that we have this configuration, and it seems to work fine:

      {
        "presets": ["es2015", "react"],
        "plugins": [
          "transform-es3-member-expression-literals",
          "transform-es3-property-literals",
          ["transform-es2015-classes", {"loose": true}],
          "add-module-exports",
          "transform-es2015-modules-commonjs",
          "transform-jscript"
        ]
      }

Note, that add-module-exports is unofficial plugin, that fixes new behavior with export. See this. Also note, that transform-jscript is for nasty issues with older browsers as well, but is not obligatory. It wasn't enabled by default in v5, and Flummox worked fine.

Pavek commented 8 years ago

I will test now ["transform-es2015-classes", {"loose": true}] option from the config @pycbouh kindly shared -- this one seems to be relevant for this issue.

voronianski commented 8 years ago

@Pavek @pycbouh thanks guys!

Pavek commented 8 years ago

@voronianski I'm confident that the following config fixes this issue:

{
    "presets": [
        "es2015-loose",
        "react",
        "stage-0"
    ],
    "plugins": [["transform-es2015-classes", {"loose": true}]]
} 

Note: the code generated for addons/FluxComponent.js is different than present in 3.6.5, so apparently "es2015-loose" is not 'loose-enough' and plugins do more.

voronianski commented 8 years ago

@Pavek thanks, then I'll add necessary plugins to .babelrc instead of es2015 presets later today.

YuriSizov commented 8 years ago

@voronianski I'm pretty sure ["transform-es2015-classes", {"loose": true}] complements es2015 preset, and not replaces it.

And please consider adding

"transform-es3-member-expression-literals",
"transform-es3-property-literals"

for IE8 support (#272).

voronianski commented 8 years ago

@Pavek @pycbouh that's weird but update to ["transform-es2015-classes", {"loose": true}] causes a lot of tests to fail.. investigating.

Pavek commented 8 years ago

@voronianski Any update regarding this? Thanks.

voronianski commented 8 years ago

@Pavek @pycbouh for now I cannot make tests pass when loose within ["transform-es2015-classes", {"loose": true}] is enabled, async/await methods on class instance are not transpiled properly.

YuriSizov commented 8 years ago

@voronianski Did you keep es2015 preset? It seems to transpile async functions, but only with an appropriate syntax plugin.

Also, if you've used es1015-loose, it has settings to disable regenerator plugin from creating async methods when it otherwise would.

voronianski commented 8 years ago

@pycbouh I've checked several setups once more and after chat in Babel's Slack account I'm aware that it's a known bug in Babel that will be fixed in next version. Hopefully it will be asap.

2015-12-06 18 34 43
voronianski commented 8 years ago

@pycbouh @Pavek good news! new version was published several seconds ago and this bug fixed - https://github.com/babel/babel/pull/3135

voronianski commented 8 years ago

@Pavek @pycbouh added in 3.6.6

YuriSizov commented 8 years ago

Thanks!

Pavek commented 8 years ago

Thanks