Failed to load in IE
ipip2005 opened this issue · 13 comments
This extra comma and some other syntax errors are causing script loading errors in IE. We're not able to use the library on IE browser.
When loading demo page on IE locally, also seen this error
This module is published as ES 2018 (see tsconfig.json), so you'll need to transpile it yourself to ES5, so that it would work in IE - that extra comma is just one example of the new syntax not supported by IE. The demo app transpiles the module using webpack and babel, and it did work in IE11 the last time I checked.
If you use webpack, remember that it does not transpile node_modules by default, so you'll need to "include" __dirname + '/node_modules/visual-dom-diff/' in babel-loader in your webpack config.
@gkubisa Thanks for explanation! Is it possible to publish this module in other (multiple) targets? It doesn't need to change es2018, just add another one that works on IE by default? Maybe the default entry is es2018 but we can still import from a different lib folder to load IE compatible modules?
I'm working in a mono repo where it's extremely hard to modify webpack configuration due to the fact how our tool chain is used.
It would be technically possible to publish for multiple targets, however, I don't want to do it because it would add complexity just for the sake of supporting IE11 by default, while a good general solution already exists.
I'm working in a mono repo where it's extremely hard to modify webpack configuration due to the fact how our tool chain is used.
We're in a similar situation now with one of our older products and are currently updating our build system to support ES6 in node_modules. It is very hard but it also seems inevitable, as the number of ES6-only modules is bound to increase over time.
If updating the build system is not an option for you at this time, a quick workaround could be to set up a new module which simply depends on visual-dom-diff, transpiles it and publishes as ES5.
@gkubisa Not sure if your vision of the library doesn't include IE at all. I still feel supporting IE by default is good for more adoption from teams like us whose product still have major usage on IE browser.
I could try your suggestion but I'm more worried about IE compatibility other than only ES6 conventions.
If this library is supposed to be IE compatible, I still think providing multiple targets is better to be supported so it's easier for more teams to adopt. What additional maintenance should be done if we enable a new target? And what can the community help with?
To be honest, I didn't make any serious effort to support IE11, as we ended up using this lib in nodejs with jsdom. IE11 support would certainly be nice to have and while I don't currently have time to implement it, I will accept a PR.
Here's what I think needs to be done:
- provide a fallback for getAttributeNames - to avoid the need for a polyfill
- stop using Symbol - to avoid the need for a polyfill
- stop using
for ofloops - to avoid code bloat and performance impact of the transpiled version - discover and avoid any other features that require polyfills - I don't think there are any more
- change the TypeScript compilation target to ES5
- create a benchmark - to ensure that performance is not negatively affected by the changes - I care mostly about v8/nodejs.
Would you like to do this work, @ipip2005 ? Let me know, if you have any questions or suggestions regarding the above.
I'm not familiar with some areas but i would like to have a try.
When I'm playing with the library locally, I encountered some issues after I pull latest master and finished rush install
-
When I run
npm run build. It failed at prettier step:
-
When I run
npm run start. The demo page launches but it's empty in the "Diff" box.
Below is my node and npm versions:
Is it same for you or I missed something?
It looks like a problem with the patterns passed to prettier on Windows. I think they might need to be updated to use double quotes " instead of single quotes ', see LBRYFoundation/lbry-wunderbot#154.
I'm doing some benchmark stuff, but I'm not sure if I should integrate benchmark to the project, and how.
So right now I'm just putting numbers
I took some sample data from diff.test.ts unit tests and executed benchmark in node environment
This is the data for master branch
And this is the data for my ie-support branch
The numbers are quite close
Nice one! Please add the benchmark to the ./scripts directory.
Added benchmark to ./scripts and added npm run benchmark to invoke t quickly