Skip to content
This repository has been archived by the owner on Feb 12, 2022. It is now read-only.

Adds a flag for Array.prototype method nested optimized functions #2404

Conversation

trueadm
Copy link
Contributor

@trueadm trueadm commented Aug 9, 2018

Release notes: adds a arrayNestedOptimizedFunctionsEnabled flag to enable nested optimized functions derived from Array.prototype methods (like map) and Array.from

This PR puts the existing (unstable) work for Array prototype methods behind a flag. The flag is enabled by default in React and serializer tests.

@@ -363,6 +363,7 @@ function runTest(name, code, options: PrepackOptions, args) {
internalDebug: true,
serialize: true,
uniqueSuffix: "",
arrayNestedOptimizedFunctionsEnabled: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

This I find confusing. A better way would be to have tests that need it and work carry some special annotation like // arrayNestedOptimizedFunctionsEnabled.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought it would be better to have it on by default in tests so we can catch bugs earlier, but I don't feel strongly and can change this.

Copy link
Contributor

Choose a reason for hiding this comment

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

And once not on by default in the serializer, can you also add the test from #2398 as a regression test?

@NTillmann
Copy link
Contributor

LGTM. Can you also add the test from #2398 as a future regression test? It passes today without the flag being set.

@trueadm
Copy link
Contributor Author

trueadm commented Aug 10, 2018

@NTillmann That test was already added and fixed as part of another PR.

Copy link

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

trueadm is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

facebook-github-bot pushed a commit that referenced this pull request Aug 15, 2018
Summary:
Fixes #2151 #2222 #2279 #2393 #2399 #2404 #2411 #2414 #2415
Added a fuzz testing tool
Added test cases
Turn crash in JSON.stringify into a diagnostic
Adds a `arrayNestedOptimizedFunctionsEnabled` flag to enable nested optimized functions derived from Array.prototype methods (like `map`) and Array.from
Refactor assignment on partial or possibly deleted property
Rewrote the joining logic to always do a full join at every join point
Removed last remnants of delayUnsupportedRequires

Reviewed By: cblappert

Differential Revision: D9349841

fbshipit-source-id: 74a16dbb015777d59d23fdfde77abbe2489c292a
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants