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

doc: history shows wrong version for conditional package export support #36162

Closed
1 task done
gpoole opened this issue Nov 18, 2020 · 5 comments · Fixed by #42339
Closed
1 task done

doc: history shows wrong version for conditional package export support #36162

gpoole opened this issue Nov 18, 2020 · 5 comments · Fixed by #42339
Labels
confirmed-bug Issues with confirmed bugs. doc Issues and PRs related to the documentations. esm Issues and PRs related to the ECMAScript Modules implementation.

Comments

@gpoole
Copy link
Contributor

gpoole commented Nov 18, 2020

📗 API Reference Docs Problem

  • Version: 12.16.0
  • Platform: All
  • Subsystem: loader

Location

Modules: Packages

Affected URL(s):

Description

Under the "history" section, the docs say that conditional export support was added and also unflagged in 12.16, which doesn't seem to be the case. 12.17.0 appears to be the first version that supports conditional exports and I can't get either conditional exports or the --experimental-conditional-exports flag to work in 12.16.0.

image


  • I would like to work on this issue and
    submit a pull request.
@gpoole gpoole added the doc Issues and PRs related to the documentations. label Nov 18, 2020
@aduh95 aduh95 added the esm Issues and PRs related to the ECMAScript Modules implementation. label Nov 18, 2020
@aduh95
Copy link
Contributor

aduh95 commented Nov 18, 2020

Conditional exports were added in #29978 and the flag was removed in #31001. It looks like that both PRs were indeed backported to v12.16.0. Maybe are you using another feature that was not backported until v12.17.0? Can you share a repro code that shows the feature is missing?

@aduh95 aduh95 added the stalled Issues and PRs that are stalled. label Dec 23, 2020
@github-actions
Copy link
Contributor

This issue/PR was marked as stalled, it will be automatically closed in 30 days. If it should remain open, please leave a comment explaining why it should remain open.

@gpoole
Copy link
Contributor Author

gpoole commented Dec 23, 2020

I've created a reproduction here https://github.com/gpoole/node-missing-export-map. While I was doing that I think I realised the problem is actually that the conditional exports do work in 12.16, but only if the --experimental-modules option is set. So the output I get from the sample is:

12.16 with node index.js: missing map hello
12.17 with node index.js: common hello
12.16 with node --experimental-modules index.js: common hello

It seems that conditional exports are tied to module support. I think this makes sense, but I think the history section doesn't make it clear and to me it says that 12.16 uses conditional exports without any flags, which it doesn't.

@aduh95
Copy link
Contributor

aduh95 commented Mar 21, 2021

Thanks for the clarification, it's probably worth clarifying the documentation indeed. Do you see the same bahavior for package self-reference in Node.js v12.16.0?

@aduh95 aduh95 added confirmed-bug Issues with confirmed bugs. and removed stalled Issues and PRs that are stalled. labels Mar 21, 2021
@gpoole
Copy link
Contributor Author

gpoole commented Mar 22, 2021

Yes, I see the same behaviour. After adding reference.cjs to the inner package I see the following:

cd node_modules/inner
node reference.cjs
# missing map hello
node --experimental-modules reference.cjs
# common hello
# (node:36203) ExperimentalWarning: The ESM module loader is experimental.

@meixg meixg closed this as completed Mar 15, 2022
gpoole added a commit to gpoole/node that referenced this issue Mar 15, 2022
gpoole added a commit to gpoole/node that referenced this issue Mar 15, 2022
nodejs-github-bot pushed a commit that referenced this issue Mar 26, 2022
Fixes: #36162

PR-URL: #42339
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
juanarbol pushed a commit that referenced this issue Apr 4, 2022
Fixes: #36162

PR-URL: #42339
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
juanarbol pushed a commit to juanarbol/node that referenced this issue Apr 5, 2022
Fixes: nodejs#36162

PR-URL: nodejs#42339
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
juanarbol pushed a commit that referenced this issue Apr 6, 2022
Fixes: #36162

PR-URL: #42339
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
xtx1130 pushed a commit to xtx1130/node that referenced this issue Apr 25, 2022
Fixes: nodejs#36162

PR-URL: nodejs#42339
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
juanarbol pushed a commit that referenced this issue May 31, 2022
Fixes: #36162

PR-URL: #42339
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
danielleadams pushed a commit that referenced this issue Jun 27, 2022
Fixes: #36162

PR-URL: #42339
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this issue Jul 11, 2022
Fixes: #36162

PR-URL: #42339
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this issue Jul 31, 2022
Fixes: #36162

PR-URL: #42339
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
guangwong pushed a commit to noslate-project/node that referenced this issue Oct 10, 2022
Fixes: nodejs/node#36162

PR-URL: nodejs/node#42339
Reviewed-By: Jacob Smith <jacob@frende.me>
Reviewed-By: Geoffrey Booth <webadmin@geoffreybooth.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. doc Issues and PRs related to the documentations. esm Issues and PRs related to the ECMAScript Modules implementation.
Projects
None yet
3 participants