Skip to content

module: do not warn when accessing __esModule of unfinished exports - #33048

Closed
addaleax wants to merge 1 commit into
nodejs:masterfrom
addaleax:circular-require-esmodule-allow
Closed

module: do not warn when accessing __esModule of unfinished exports#33048
addaleax wants to merge 1 commit into
nodejs:masterfrom
addaleax:circular-require-esmodule-allow

Conversation

@addaleax

Copy link
Copy Markdown
Member

Since this property access is performed by generated code, and not
used for accessing the actual exports of a module (and because
transpilers generally define it as the first key of module.exports
when it is present), it should be okay to allow it.

Refs: #29935
Fixes: #33046

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines

Since this property access is performed by generated code, and not
used for accessing the actual exports of a module (and because
transpilers generally define it as the first key of `module.exports`
when it *is* present), it should be okay to allow it.
Refs: nodejs#29935Fixes: nodejs#33046
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@guybedfordguybedford left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems sensible.

@addaleaxaddaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Apr 24, 2020
@nodejs-github-bot

nodejs-github-bot commented Apr 24, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/30964/ (:heavy_check_mark:)

@addaleax

Copy link
Copy Markdown
MemberAuthor

Landed in 38998d8

addaleax added a commit that referenced this pull request Apr 27, 2020
Since this property access is performed by generated code, and not
used for accessing the actual exports of a module (and because
transpilers generally define it as the first key of `module.exports`
when it *is* present), it should be okay to allow it.
Refs: #29935Fixes: #33046
PR-URL: #33048
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
@addaleax
addaleax deleted the circular-require-esmodule-allow branch April 27, 2020 17:33
BethGriggs pushed a commit that referenced this pull request Apr 27, 2020
Since this property access is performed by generated code, and not
used for accessing the actual exports of a module (and because
transpilers generally define it as the first key of `module.exports`
when it *is* present), it should be okay to allow it.
Refs: #29935Fixes: #33046
PR-URL: #33048
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
@BethGriggsBethGriggs mentioned this pull request Apr 27, 2020
BethGriggs added a commit that referenced this pull request Apr 27, 2020
Notable changes:
- deps: upgrade openssl sources to 1.1.1g (Hassaan Pasha)
[#32971](#32971)
- module: do not warn when accessing `\_\_esModule` of unfinished exports (Anna Henningsen)
[#33048](#33048)
- stream:
- don't wait for close on legacy streams (Robert Nagy)
[#33058](#33058)
- pipeline should only destroy un-finished streams (Robert Nagy)
[#32968](#32968)
PR-URL: #33103
BethGriggs added a commit that referenced this pull request Apr 28, 2020
Notable changes:
- deps: upgrade openssl sources to 1.1.1g (Hassaan Pasha)
[#32971](#32971)
- module: do not warn when accessing `\_\_esModule` of unfinished exports (Anna Henningsen)
[#33048](#33048)
- stream:
- don't wait for close on legacy streams (Robert Nagy)
[#33058](#33058)
- pipeline should only destroy un-finished streams (Robert Nagy)
[#32968](#32968)
PR-URL: #33103
BethGriggs pushed a commit that referenced this pull request Apr 28, 2020
Since this property access is performed by generated code, and not
used for accessing the actual exports of a module (and because
transpilers generally define it as the first key of `module.exports`
when it *is* present), it should be okay to allow it.
Refs: #29935Fixes: #33046
PR-URL: #33048
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Zeyu Yang <himself65@outlook.com>
BethGriggs added a commit that referenced this pull request Apr 28, 2020
Notable changes:
- deps: upgrade openssl sources to 1.1.1g (Hassaan Pasha)
[#32971](#32971)
- doc: add juanarbol as collaborator (Juan José Arboleda)
[#32906](#32906)
- http: doc deprecate abort and improve docs (Robert Nagy)
[#32807](#32807)
- module: do not warn when accessing `__esModule` of unfinished exports
(Anna Henningsen) [#33048](#33048)
- n-api: detect deadlocks in thread-safe function (Gabriel Schulhof)
[#32860](#32860)
- src: deprecate embedder APIs with replacements (Anna Henningsen)
[#32858](#32858)
- stream:
- don't emit end after close (Robert Nagy)
[#33076](#33076)
- don't wait for close on legacy streams (Robert Nagy)
[#33058](#33058)
- pipeline should only destroy un-finished streams (Robert Nagy)
[#32968](#32968)
- vm: add importModuleDynamically option to compileFunction (Gus Caplan)
[#32985](#32985)
PR-URL: #33103
BethGriggs added a commit that referenced this pull request Apr 29, 2020
Notable changes:
- deps: upgrade openssl sources to 1.1.1g (Hassaan Pasha)
[#32971](#32971)
- doc: add juanarbol as collaborator (Juan José Arboleda)
[#32906](#32906)
- http: doc deprecate abort and improve docs (Robert Nagy)
[#32807](#32807)
- module: do not warn when accessing `__esModule` of unfinished exports
(Anna Henningsen) [#33048](#33048)
- n-api: detect deadlocks in thread-safe function (Gabriel Schulhof)
[#32860](#32860)
- src: deprecate embedder APIs with replacements (Anna Henningsen)
[#32858](#32858)
- stream:
- don't emit end after close (Robert Nagy)
[#33076](#33076)
- don't wait for close on legacy streams (Robert Nagy)
[#33058](#33058)
- pipeline should only destroy un-finished streams (Robert Nagy)
[#32968](#32968)
- vm: add importModuleDynamically option to compileFunction (Gus Caplan)
[#32985](#32985)
PR-URL: #33103
BethGriggs added a commit that referenced this pull request Apr 29, 2020
Notable changes:
- deps: upgrade openssl sources to 1.1.1g (Hassaan Pasha)
[#32971](#32971)
- doc: add juanarbol as collaborator (Juan José Arboleda)
[#32906](#32906)
- http: doc deprecate abort and improve docs (Robert Nagy)
[#32807](#32807)
- module: do not warn when accessing `__esModule` of unfinished exports
(Anna Henningsen) [#33048](#33048)
- n-api: detect deadlocks in thread-safe function (Gabriel Schulhof)
[#32860](#32860)
- src: deprecate embedder APIs with replacements (Anna Henningsen)
[#32858](#32858)
- stream:
- don't emit end after close (Robert Nagy)
[#33076](#33076)
- don't wait for close on legacy streams (Robert Nagy)
[#33058](#33058)
- pipeline should only destroy un-finished streams (Robert Nagy)
[#32968](#32968)
- vm: add importModuleDynamically option to compileFunction (Gus Caplan)
[#32985](#32985)
PR-URL: #33103
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.moduleIssues and PRs related to the module subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Node v14.0.0 : Accessing non-existent property '__esModule' of module exports inside circular dependency

5 participants

@addaleax@nodejs-github-bot@jasnell@guybedford@himself65