Skip to content

Conversation

@addaleax
Copy link
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
Collaborator

Copy link
Contributor

@guybedfordguybedford left a comment

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
Copy link
Collaborator

nodejs-github-bot commented Apr 24, 2020

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

@addaleax
Copy link
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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Zeyu Yang <[email protected]>
@addaleaxaddaleax 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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Zeyu Yang <[email protected]>
@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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Zeyu Yang <[email protected]>
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) 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 <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Zeyu Yang <[email protected]>
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