Deprecate IE8 support#2141
Merged
Merged
Conversation
guybedford
force-pushed
the
deprecate-legacy
branch
from
April 30, 2018 20:57
5140268 to
e86e25e
Compare
guybedford
force-pushed
the
deprecate-legacy
branch
from
May 22, 2018 18:54
e86e25e to
20f6210
Compare
lukastaegert
approved these changes
May 28, 2018
lukastaegert
left a comment
lukastaegert
left a comment
Member
There was a problem hiding this comment.
Looks good! I have been thinking about waiting for 1.0.0 myself but considering we do not have something like an IE8 E2E test on browserstack or similar and there is always a chance we already broke IE8 compatibility anyway with some other feature I would just put this into 0.60.0 and communicate this as a breaking change. If there is a lot of outcry, it should not be too difficult to revert this. Otherwise we would always have to maintain a separate branch.
lukastaegert
force-pushed
the
deprecate-legacy
branch
from
May 30, 2018 04:47
20f6210 to
9078957
Compare
Closed
elas7
added a commit
to elas7/react
that referenced
this pull request
Aug 9, 2018
The `legacy` option was added to the Rollup build system (for www builds) in react#11469 because the internal transforms break on getters. As of version 0.60.0 of Rollup, `legacy` support is removed ([source](rollup/rollup#2141), [changelog](https://github.com/rollup/rollup/blob/master/CHANGELOG.md#0600)). This PR adds a custom Rollup plugin to replicate the `legacy` behaviour. Note that the `legacy` option preformed several transformations to support IE8. The custom Rollup plugin only replicates the replacement of getters ([original code here](https://github.com/rollup/rollup/blob/349677ceee9d4bbccb5b2f72e653270cb2b0ce51/src/ast/variables/NamespaceVariable.ts#L63)). Also note that getters are only generated in a very specific case by Rollup. That is, when doing namespace imports (`import * as name from 'origin'`) and one of the imported variables is reassigned at some point (an example of this can be found in the [old Rollup tests](https://github.com/rollup/rollup/tree/349677ceee9d4bbccb5b2f72e653270cb2b0ce51/test/form/samples/legacy-getter)). At this time, the only getter generated by React code is `ReactDOMEventListener._enabled`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
As discussed in #2135.
This could potentially be something we mark as a 1.0 milestone as well.