Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove unnecessary lines for hasBadMapPolyfill issue for rollup #16231

Merged
merged 1 commit into from Mar 22, 2020
Merged

Remove unnecessary lines for hasBadMapPolyfill issue for rollup #16231

merged 1 commit into from Mar 22, 2020

Conversation

iAziz786
Copy link
Contributor

The removed lines in this PR were added by @gaearon in #11745. The reason was a tree-shaking bug in the rollup.

According to @lukastaegert, this bug has been fixed and added to rollup in v1.14.0

I was not sure whether to use the rollup version 1.14.0 or to upgrade it to the latest, so I updated it the latest (1.17.0).

Note: One odd observation was that I ran the test with version 0.52.1 and they all still seem to pass.

@sizebot
Copy link

sizebot commented Jul 28, 2019

Details of bundled changes.

Comparing: 75ab53b...02382a1

react

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
react.development.js -0.2% -0.3% 116.91 KB 116.66 KB 29.33 KB 29.24 KB UMD_DEV
react.profiling.min.js 0.0% 0.0% 15.23 KB 15.23 KB 5.61 KB 5.61 KB UMD_PROFILING
react.development.js -0.3% -0.5% 71.94 KB 71.7 KB 18.88 KB 18.79 KB NODE_DEV
React-dev.js -0.4% -0.6% 69.76 KB 69.51 KB 17.93 KB 17.83 KB FB_WWW_DEV
React-prod.js 0.0% -0.0% 17.38 KB 17.38 KB 4.54 KB 4.54 KB FB_WWW_PROD
React-profiling.js 0.0% -0.0% 17.38 KB 17.38 KB 4.54 KB 4.54 KB FB_WWW_PROFILING

react-native-renderer

File Filesize Diff Gzip Diff Prev Size Current Size Prev Gzip Current Gzip ENV
ReactNativeRenderer-prod.js 0.0% -0.0% 271.99 KB 271.99 KB 46.7 KB 46.7 KB RN_OSS_PROD
ReactFabric-profiling.js 0.0% -0.0% 274.53 KB 274.53 KB 47.24 KB 47.24 KB RN_OSS_PROFILING
ReactFabric-dev.js -0.0% -0.1% 733.63 KB 733.35 KB 154.96 KB 154.87 KB RN_FB_DEV
ReactNativeRenderer-dev.js -0.0% -0.1% 720.92 KB 720.64 KB 152.5 KB 152.41 KB RN_OSS_DEV
ReactFabric-profiling.js 0.0% -0.0% 274.53 KB 274.53 KB 47.25 KB 47.25 KB RN_FB_PROFILING
ReactNativeRenderer-dev.js -0.0% -0.1% 721.01 KB 720.73 KB 152.54 KB 152.46 KB RN_FB_DEV
ReactNativeRenderer-prod.js 0.0% -0.0% 271.99 KB 271.99 KB 46.71 KB 46.71 KB RN_FB_PROD
ReactFabric-dev.js -0.0% -0.1% 733.53 KB 733.25 KB 154.92 KB 154.82 KB RN_OSS_DEV

Generated by 🚫 dangerJS

@iAziz786
Copy link
Contributor Author

iAziz786 commented Jul 28, 2019

The build was failing because of the rollup version upgrade so I revert back to the version it was before. Interestingly, all tests are passing. Apparently It was already fixed in the current version. 🤔

@stale
Copy link

stale bot commented Jan 10, 2020

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution.

@stale stale bot added the Resolution: Stale Automatically closed due to inactivity label Jan 10, 2020
@lukastaegert
Copy link

I do not think this is stale but it depends on finally updating to the latest Rollup version, i.e. merging #15037. Then this change is a no-brainer.

@stale stale bot removed the Resolution: Stale Automatically closed due to inactivity label Jan 10, 2020
@gaearon
Copy link
Collaborator

gaearon commented Feb 28, 2020

We bumped the version. Mind rebasing?

@iAziz786
Copy link
Contributor Author

Sure. I'll let you know once done.

@codesandbox-ci
Copy link

codesandbox-ci bot commented Feb 28, 2020

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 7bd2a61:

Sandbox Source
relaxed-galileo-vuq5k Configuration

@sizebot
Copy link

sizebot commented Feb 28, 2020

No significant bundle size changes to report.

Size changes (experimental)

Generated by 🚫 dangerJS against 7bd2a61

@sizebot
Copy link

sizebot commented Feb 28, 2020

No significant bundle size changes to report.

Size changes (stable)

Generated by 🚫 dangerJS against 7bd2a61

@iAziz786
Copy link
Contributor Author

I have made the changes. @gaearon Running yarn test-prod is breaking in my system but looks like it's okay with CircleCI.

Are we good here?

@iAziz786
Copy link
Contributor Author

I had to rebase it again because of some file name changes in this PR

@gaearon gaearon merged commit 31a9e39 into facebook:master Mar 22, 2020
@gaearon
Copy link
Collaborator

gaearon commented Mar 22, 2020

Thanks

@iAziz786 iAziz786 deleted the rollup-try-statement-deoptimization branch March 23, 2020 02:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants