Revert "Remove old IE polyfill code" #10483
Conversation
**what is the change?:** This reverts facebook@0b220d0 Which was part of facebook#10238 **why make this change?:** When trying to sync the latest React with FB's codebase, we had failing integration tests. It looks like they are running with an old version of Chrome and there is something related to file upload that fails when these polyfills are missing. For now we'd like to revert this to unblock syncing, but it's worth revisiting this change to try and add some polyfill for FB and remove it from React, or to fix whatever the specific issue was with our file upload error. **test plan:** `yarn test` and also built and played with the `dom` and `packaging` fixtures
flarnie
requested review from
sophiebits,
bvaughn,
jquense,
nhunzaker and
aweary
|
For folks reviewing - this should be the reverse of 7659334 |
|
looks like a clean revert. It'd be great to have some more info about what is failing. I realize that's all internal; Just want to avoid rehashing the same paths over and over, or trying to clean up code in the dark. Avoiding regressions or behavior changes in these areas is tough enough without seeing feedback on what is breaking (or on what version of chrome, etc) :) |
|
In Chrome 56.0.2924.87 (linux), we were seeing that |
| getTargetInstFunc = getTargetInstForInputEventPolyfill; | ||
| handleEventFunc = handleEventsForInputEventPolyfill; | ||
| } | ||
| } else if (shouldUseClickEvent(targetNode)) { |
There was a problem hiding this comment.
it'd be nice to keep the change removing the click behavior for checkable inputs. I can see if I can just pull those bits out. I guess we have no reason to think this isn't the cause here.
There was a problem hiding this comment.
Sure - if you make a follow-up PR I can verify that it doesn't cause the issue with our integration tests.
Also +1 to what @spicyj said - we are happy to help find a repro case that is not just our internal tests. Thanks for your help on this!
There was a problem hiding this comment.
perfect i'll try and get something up. Thinking more on it, it should be fine to stick some of this in a patch release anyway, given it's supposed just be clearing out unused code (why are browsers so hard).
thanks yall
what is the change?:
This reverts 0b220d0
Which was part of #10238
why make this change?:
When trying to sync the latest React with FB's codebase, we had failing
integration tests.
It looks like they are running with an old version of Chrome and there
is something related to file upload that fails when these polyfills are
missing.
For now we'd like to revert this to unblock syncing, but it's worth
revisiting this change to try and add some polyfill for FB and remove it
from React, or to fix whatever the specific issue was with our file
upload error.
test plan:
yarn testand also built and played with thedomandpackagingfixtures: