Skip to content

Conversation

guybedford
Copy link
Contributor

As discussed in #2135.

This could potentially be something we mark as a 1.0 milestone as well.

Copy link
Member

@lukastaegert lukastaegert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 lukastaegert added this to the 0.60.0 milestone May 30, 2018
@lukastaegert lukastaegert merged commit 9078957 into master May 30, 2018
@lukastaegert lukastaegert deleted the deprecate-legacy branch May 30, 2018 05:37
@lukastaegert lukastaegert mentioned this pull request Jul 20, 2018
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 facebook#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`.
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.

2 participants