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

esm: refactor createDynamicModule() #27809

Merged
merged 1 commit into from
May 24, 2019
Merged

esm: refactor createDynamicModule() #27809

merged 1 commit into from
May 24, 2019

Conversation

cjihrig
Copy link
Contributor

@cjihrig cjihrig commented May 22, 2019

This commit refactors createDynamicModule() for readability:

  • The map() callback functions are named and moved to a higher scope.
  • The two export map() loops are combined.
  • JSON.stringify() is only called once per import.
Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot
Copy link
Collaborator

@devsnek devsnek added esm Issues and PRs related to the ECMAScript Modules implementation. experimental Issues and PRs related to experimental features. labels May 22, 2019
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented May 23, 2019

CI: https://ci.nodejs.org/job/node-test-pull-request/23287/

EDIT(cjihrig): CI was yellow.

@ZYSzys ZYSzys added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 23, 2019
This commit refactors createDynamicModule() for readability:

- The map() callback functions are named and moved to a higher
  scope.
- The two export map() loops are combined.
- JSON.stringify() is only called once per import.

PR-URL: nodejs#27809
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@cjihrig cjihrig merged commit 3293bbf into nodejs:master May 24, 2019
@cjihrig cjihrig deleted the dyn-mod branch May 24, 2019 12:36
targos pushed a commit that referenced this pull request May 28, 2019
This commit refactors createDynamicModule() for readability:

- The map() callback functions are named and moved to a higher
  scope.
- The two export map() loops are combined.
- JSON.stringify() is only called once per import.

PR-URL: #27809
Reviewed-By: Gus Caplan <me@gus.host>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@targos targos mentioned this pull request Jun 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. esm Issues and PRs related to the ECMAScript Modules implementation. experimental Issues and PRs related to experimental features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.