neherlab / covid19_scenarios

Models of COVID-19 outbreak trajectories and hospital demand
https://covid19-scenarios.org
MIT License
1.36k stars 354 forks source link

Fix rendering in Internet Explorer #20

Closed ivan-aksamentov closed 4 years ago

ivan-aksamentov commented 4 years ago

We target to support browsers starting with Internet Explorer 10, and other browsers of the same era.

Currently Internet Explorer is broken, despite us transpiling the code and having many polyfills

malikmukhtar commented 4 years ago

@ivan-aksamentov Can I work on this?

lordrip commented 4 years ago

The problem with development is because there's some code that is not being transpiled to ES5, if you execute yarn dev and open .build/development/web/content/main.js at line 1000 you'll notice this: const { Html5Entities } = __webpack_require__("./node_modules/html-entities/index.js"); and this is ES2015 and is not supported by our beloved IE11.

This should be fixed by editing .browserlistrc file but unfortunately I couldn't make it work yet.

ivan-aksamentov commented 4 years ago

@malikmukhtar Sure, go ahead.

@lordrip , dev stuff does not need to be compatible with IE. Neither transpilation would help. There is just too much hairy stuff injected by various dev tools. In any case, we assume that our developers are competent enough to download any of 5 latest versions of Firefox or Chrome :)

Production is transpiled differently, and mostly correctly. There are some issues with polyfills: for example my IE10 cannot find Map, even though we have a polyfill for it.

If you have some time resources, please concentrate your efforts on yarn prod:watch and opening http://localhost:8080 in IE. (for example on a Virtual Machine)

I am planing to setup browserstack bot soon(-ish) Let me know if you need anything, folks.

lordrip commented 4 years ago

@ivan-aksamentov sure thing, I'll do, in order hand, and please excuse myself if this was already asked, but, CRA script shouldn't help us with all of this? I mean, developing and building for Windows (and also for IE)?.

In any case, I'll concentrate in your direction

ivan-aksamentov commented 4 years ago

@lordrip we fixed development on Windows today (hopefully) in #38, towards the end.

Regarding the app not working in IE, CRA would not help at all, as every app uses different stuff and requires different polyfills. Plus, our build system have so much stuff, that changing to CRA right now would be a hiuge adventure. By the way, by CRA you mean create-react-app and their scripts package, right?)

What causesMap error, it is probably a strain library somewhere. Or maybe something tries to use Map() before polyfills are loaded. I don't know.

In any case I may be wrong, and if you find a solution, with CRA or not - submit a pull request. Our team would be very grateful!

ivan-aksamentov commented 4 years ago

Related #134

lordrip commented 4 years ago

Hi @ivan-aksamentov, I've migrated build part to CRA, added polyfills for IE and it works. There's still more work remaining regarding the other tools that you have in place. I'll work a little bit more on this after work and I'll share the branch to check.

gj262 commented 4 years ago

Hi @lordrip, I've downloaded a VM and can double check your changes for you when/if you want. Cheers, -Gavin

lordrip commented 4 years ago

Hi @gj262 thanks a lot for the help. I've uploaded a PR here https://github.com/neherlab/covid19_scenarios/pull/188, but I need to check how now works to be able to deploy a preview.

In other hand, I have a warning from snyk but I'm not able to log in and check what is it all about.

In any case, I'll keep updating the PR to fix this issues.

lordrip commented 4 years ago

@gj262 the branch is uploaded, I think that you can test now here

🔍 Inspect: https://zeit.co/covid19-scenarios/covid19-scenarios/h7vgy31b1 ✅ Preview: https://covid19-scenarios-git-fork-lordrip-cra.covid19-scenarios.now.sh

or build locally :) thanks in advance!

gj262 commented 4 years ago

Hi Ricardo, @lordrip, I'll give #188 a run today. I need to repair my windows env first though. Looking @saucelabs. It would be good to confirm that #188 fixes the issue, however the PR is quite large and touches on a lot of areas that are not related to getting IE10 working. Probably they were required to get CRA working. I'd love to see a PR that just fixes that issue. I started poking around at index.polyfilled.ts to try to understand why Map might not be loaded but my webpack is a little dated.

lordrip commented 4 years ago

Yes, unfortunately, since the project has TypeScript not directly together with the build, there's several situations that needs to be fixed in order to be compiled with CRA.

My reasoning behind this is since I couldn't make it work directly with polyfills, then I've tried to migrate it CRA. In any case, I understand it, I'll decline the PR and I'll be waiting in case I can help with something else on another issue.

Keep the good work and thanks for the help 💪💪💪 :)

gj262 commented 4 years ago

Some progress! 0008screenshot

gj262 commented 4 years ago

The Map error is caused by loading 'map.prototype.tojson' before the main entry point.

https://github.com/neherlab/covid19_scenarios/blob/29ef1877d7dcede9313bcfcd5e1766fb446963d6/config/webpack/webpack.client.babel.ts#L87

Given that redux is not really used in this project, it is probably safe to just remove that.

There are other missing polyfills that I need to work through to get a working IE10.

gj262 commented 4 years ago

Something in the tsc/babel/webpack chain is leaving/putting a 'const' in the output bundle. Don't know why. 😢 0002screenshot

ivan-aksamentov commented 4 years ago

@lordrip @gj262 I am handling this. I moved redux visualizers below polyfills, so the Map problem is solved, but there are couple more errors on IE10.

We may opt-out from selective polyfilling and code-splitting and just include the whole core-js as CRA does, but I don't want to increase bundle size because of 1% of people who are still on IE. Neither I want to revamp the entire build system because of this.

gj262 commented 4 years ago

I changed the polyfiller in my WIP load all core-js to get to the point above.

    typeof Object.assign === 'undefined' &&
    import(/* webpackMode: "lazy" */ /* webpackChunkName: "polyfills/core-js" */ 'core-js'), // prettier-ignore