Skip to content

Conversation

@ponelat
Copy link
Contributor

@ponelat ponelat commented Apr 23, 2017

For integrating this lib into other apps, that already include babel-polyfill

@ponelat ponelat requested a review from shockey April 23, 2017 15:26
@shockey
Copy link
Contributor

shockey commented Apr 23, 2017

My concern here is IE11, babel-polyfill fixed a Symbol reference error for it in UI a couple months ago.

@ponelat
Copy link
Contributor Author

ponelat commented Apr 24, 2017

Ok, so we'd need to test it in IE11 to know for sure.
The differences are in instance methods, which are NOT covered by transform-runtime.

Babel-polyfill is a big blocker for using swagger-ui as a component. Since there can only be one reference. An alternative can be: ignoring the call to babel-polyfill if one has already been called.
Not sure if that functionality is going to land in babel-polyfill, I know its mentioned there.

So tl;dr... we should either ensure our the code works with transform-runtime. Or add a workaround for having two instances. ( Assuming we want swagger-ui as a component... :D )

@webron
Copy link
Contributor

webron commented Jun 8, 2017

@ponelat @shockey ?

@ponelat
Copy link
Contributor Author

ponelat commented Jun 9, 2017

If we can lets use transform-runtime.
It means stripping out any modern instance methods. We can get a list and just prune away. Or add those polyfills manually ( ie: one-by-one on an as-needed basis ).

The alternative is to patch babel-polyfill to run multiple times ( it should be idempotent, but for some reason they decided to throw an error if there are multiple calls to it. I'm curious as to why ).

@ponelat ponelat force-pushed the feature/remove-babel-polyfill branch 3 times, most recently from 480a0dd to 4a6716f Compare June 15, 2017 14:52
@ponelat
Copy link
Contributor Author

ponelat commented Jun 15, 2017

An idea is to use https://github.com/amilajack/eslint-plugin-compat to lint the code and detect instance methods that aren't supported. It might work :D

ponelat added 2 commits June 19, 2017 18:29
This is needed for whatwg-fetch + IE11.
An alternative is to include "node_modules/whatwg-fetch" in the
transform-runtime.
But my guess is that someone is likely going to add a lib that in turn
uses Promises, without adding it to the whitelist. This is safter.
@ponelat
Copy link
Contributor Author

ponelat commented Jun 19, 2017

This now works in IE 11, so we're good for that review @shockey.
Keep in mind, that we can always fallback to using babel-polyfill in the standalone version - its the lib we're concerned with.

@ponelat
Copy link
Contributor Author

ponelat commented Jun 19, 2017

Also need swagger-api/swagger-js#1088 to work

@shockey
Copy link
Contributor

shockey commented Jun 20, 2017

LGTM for IE11:

image

@shockey shockey merged commit 55e4a12 into master Jun 20, 2017
@ponelat ponelat deleted the feature/remove-babel-polyfill branch June 20, 2017 18:38
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

Successfully merging this pull request may close these issues.

4 participants