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

Cache JestHook emitters #8746

Merged
merged 2 commits into from Aug 3, 2019

Conversation

@Connormiha
Copy link
Contributor

Connormiha commented Jul 24, 2019

Summary

Just cached JestHookSubscriber and JestHookEmitter to avoid creating new object on each getSubscriber(), getEmitter() calls.

Test plan

Copy link
Contributor

scotthovestadt left a comment

The idea makes sense. Want to take it a step further and switch the data structure of this._listeners to a Map for more perf wins?

@Connormiha

This comment has been minimized.

Copy link
Contributor Author

Connormiha commented Jul 25, 2019

The idea makes sense. Want to take it a step further and switch the data structure of this._listeners to a Map for more perf wins?

This may impair readability and convenience.

hooks.getEmitter().onTestRunComplete(results);

vs

hooks.getEmitter().get('onTestRunComplete')(results);
Copy link
Collaborator

rogeliog left a comment

LGTM

@scotthovestadt

This comment has been minimized.

Copy link
Contributor

scotthovestadt commented Aug 3, 2019

Let's make sure this gets in for the release-- just needs a changelog.

@Connormiha Connormiha force-pushed the Connormiha:cache-jest-hooks-emitters branch from 2a4d7dc to f96a3b9 Aug 3, 2019
@Connormiha

This comment has been minimized.

Copy link
Contributor Author

Connormiha commented Aug 3, 2019

Let's make sure this gets in for the release-- just needs a changelog.

done @scotthovestadt

@SimenB
SimenB approved these changes Aug 3, 2019
@SimenB SimenB merged commit cff67fd into facebook:master Aug 3, 2019
10 of 11 checks passed
10 of 11 checks passed
ci/circleci: test-jest-circus Your tests failed on CircleCI
Details
ci/circleci: lint-and-typecheck Your tests passed on CircleCI!
Details
ci/circleci: test-browser Your tests passed on CircleCI!
Details
ci/circleci: test-node-10 Your tests passed on CircleCI!
Details
ci/circleci: test-node-11 Your tests passed on CircleCI!
Details
ci/circleci: test-node-6 Your tests passed on CircleCI!
Details
ci/circleci: test-node-8 Your tests passed on CircleCI!
Details
ci/circleci: test-or-deploy-website Your tests passed on CircleCI!
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
deploy/netlify Deploy preview ready!
Details
facebook.jest #20190803.1 succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.