FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

module: reduce url invocations in esm/load.js by anonrig · Pull Request #48337 · nodejs/node · GitHub

/ node Public

module: reduce url invocations in esm/load.js - #48337

Merged
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
anonrig:improve-esm-loadjs
Jun 7, 2023
Merged

module: reduce url invocations in esm/load.js#48337
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
anonrig:improve-esm-loadjs

Conversation

anonrig commented Jun 5, 2023
edited
Loading

Copy link
Copy Markdown
Member

Reduces unnecessary new URL call, and removes unnecessary invokes to url.protocol

Ref: nodejs/performance#92

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/loaders
  • @nodejs/modules

nodejs-github-bot added esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run. labels Jun 5, 2023
anonrig force-pushed the improve-esm-loadjs branch from cadc77b to 724ee2e Compare June 5, 2023 01:13
anonrig force-pushed the improve-esm-loadjs branch from 724ee2e to bd035a6 Compare June 5, 2023 01:31
Comment thread lib/internal/modules/esm/load.js Outdated
Comment thread lib/internal/modules/esm/load.js Outdated
anonrig added author ready PRs that have at least one approval, no outstanding review comments, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. request-ci Add this label to start a Jenkins CI on a PR. labels Jun 5, 2023
github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 5, 2023

Copy link
Copy Markdown
Collaborator

JakobJingleheimer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I think we had talked about something similar previously, where I suggested the load hook be passed a URL instance (which we already have internally), but for some reason that I don't recall that was not good.

I think whatever that reason (something about mutability?) was, it doesn't apply here because this is internal.

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 7, 2023
nodejs-github-bot merged commit 9d936fa into nodejs:main Jun 7, 2023

Copy link
Copy Markdown
Collaborator

Landed in 9d936fa

RafaelGSS pushed a commit that referenced this pull request Jul 3, 2023
PR-URL: #48337
Refs: nodejs/performance#92
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
RafaelGSS mentioned this pull request Jul 3, 2023
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
PR-URL: nodejs#48337
Refs: nodejs/performance#92
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
Ceres6 pushed a commit to Ceres6/node that referenced this pull request Aug 14, 2023
PR-URL: nodejs#48337
Refs: nodejs/performance#92
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
ruyadorno pushed a commit that referenced this pull request Aug 29, 2023
PR-URL: #48337
Refs: nodejs/performance#92
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Jacob Smith <jacob@frende.me>
ruyadorno mentioned this pull request Aug 29, 2023
codebytere added a commit to electron/electron that referenced this pull request Sep 19, 2023
codebytere added a commit to electron/electron that referenced this pull request Sep 19, 2023
codebytere added a commit to electron/electron that referenced this pull request Sep 20, 2023
codebytere added a commit to electron/electron that referenced this pull request Sep 20, 2023
codebytere added a commit to electron/electron that referenced this pull request Sep 20, 2023
jkleinsc pushed a commit to electron/electron that referenced this pull request Sep 20, 2023
* chore: bump node in DEPS to v18.18.0

* child_process: harden against prototype pollution

nodejs/node#48726

* deps: upgrade to libuv 1.46.0

nodejs/node#49591

* module: reduce url invocations in esm/load.js

nodejs/node#48337

* Revert "test: remove test-crypto-keygen flaky designation"

nodejs/node#48652

* fix: FTBTFS in ada dep

ada-url/ada#464
ada-url/idna#31

* fix: force_colors snapshot line number

* chore: fixup patch indices

* chore: update filenames.json

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
MrHuangJser pushed a commit to MrHuangJser/electron that referenced this pull request Dec 11, 2023
* chore: bump node in DEPS to v18.18.0

* child_process: harden against prototype pollution

nodejs/node#48726

* deps: upgrade to libuv 1.46.0

nodejs/node#49591

* module: reduce url invocations in esm/load.js

nodejs/node#48337

* Revert "test: remove test-crypto-keygen flaky designation"

nodejs/node#48652

* fix: FTBTFS in ada dep

ada-url/ada#464
ada-url/idna#31

* fix: force_colors snapshot line number

* chore: fixup patch indices

* chore: update filenames.json

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
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 outstanding review comments, and a CI started. esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants


Back | FazBrowse Home | New Git URL