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

docs: refine conditional exports documentation #32098

Closed
wants to merge 39 commits into from

Conversation

guybedford
Copy link
Contributor

@guybedford guybedford commented Mar 4, 2020

This began as a bigger change of guidance but has been pulled back to a simple docs refactoring.

  • Moves exports sugar section up
  • Makes conditional exports examples using main form directly
  • Explains conditional exports sugar second
  • Adds some extra guidance on conditions usage
  • Cuts down on some unnecessary explanations to try aid readability
  • Removes incorrect statement about export main falling back to package.json main

Resolves #32107.

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

@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to the documentations. esm Issues and PRs related to the ECMAScript Modules implementation. labels Mar 4, 2020
@guybedford guybedford changed the title docs: update conditional exports recommendations docs: refine conditional exports documentation Mar 4, 2020
@guybedford
Copy link
Contributor Author

Discussing this further, early 13.x support not working isn't that bad, and it may make sense not to skew things towards this compatibility after all.

I've completely pulled this back to just be a docs refactoring in that case.

doc/api/esm.md Outdated Show resolved Hide resolved
doc/api/esm.md Outdated Show resolved Hide resolved
doc/api/esm.md Outdated Show resolved Hide resolved
doc/api/esm.md Outdated Show resolved Hide resolved
doc/api/esm.md Outdated Show resolved Hide resolved
doc/api/esm.md Outdated Show resolved Hide resolved
doc/api/esm.md Outdated Show resolved Hide resolved
doc/api/esm.md Show resolved Hide resolved
doc/api/esm.md Outdated Show resolved Hide resolved
MylesBorins and others added 9 commits March 4, 2020 18:16
Co-Authored-By: Geoffrey Booth <GeoffreyBooth@users.noreply.github.com>
Co-Authored-By: Geoffrey Booth <GeoffreyBooth@users.noreply.github.com>
Co-Authored-By: Geoffrey Booth <GeoffreyBooth@users.noreply.github.com>
Co-Authored-By: Geoffrey Booth <GeoffreyBooth@users.noreply.github.com>
Co-Authored-By: Geoffrey Booth <GeoffreyBooth@users.noreply.github.com>
Co-Authored-By: Geoffrey Booth <GeoffreyBooth@users.noreply.github.com>
Co-Authored-By: Geoffrey Booth <GeoffreyBooth@users.noreply.github.com>
Co-Authored-By: Geoffrey Booth <GeoffreyBooth@users.noreply.github.com>
@guybedford
Copy link
Contributor Author

@GeoffreyBooth thanks for the initial review - I've added some further refactorings here in 74206e7.

If you have a moment to review that would be great.

doc/api/esm.md Outdated Show resolved Hide resolved
doc/api/esm.md Outdated Show resolved Hide resolved
guybedford and others added 10 commits March 11, 2020 22:15
Co-Authored-By: Geoffrey Booth <GeoffreyBooth@users.noreply.github.com>
Co-Authored-By: Geoffrey Booth <GeoffreyBooth@users.noreply.github.com>
Co-Authored-By: Geoffrey Booth <GeoffreyBooth@users.noreply.github.com>
Co-Authored-By: Geoffrey Booth <GeoffreyBooth@users.noreply.github.com>
Co-Authored-By: Geoffrey Booth <GeoffreyBooth@users.noreply.github.com>
Co-Authored-By: Geoffrey Booth <GeoffreyBooth@users.noreply.github.com>
Co-Authored-By: Geoffrey Booth <GeoffreyBooth@users.noreply.github.com>
Co-Authored-By: Geoffrey Booth <GeoffreyBooth@users.noreply.github.com>
Co-Authored-By: Geoffrey Booth <GeoffreyBooth@users.noreply.github.com>
doc/api/esm.md Outdated Show resolved Hide resolved
addaleax pushed a commit that referenced this pull request Mar 13, 2020
Co-Authored-By: Geoffrey Booth <GeoffreyBooth@users.noreply.github.com>
PR-URL: #32098
Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
@addaleax
Copy link
Member

Landed in 04028aa

@addaleax addaleax closed this Mar 13, 2020
@addaleax addaleax deleted the doc-exports-pattern branch March 13, 2020 09:09
BridgeAR pushed a commit that referenced this pull request Mar 17, 2020
Co-Authored-By: Geoffrey Booth <GeoffreyBooth@users.noreply.github.com>
PR-URL: #32098
Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
@MylesBorins MylesBorins mentioned this pull request Mar 19, 2020
MylesBorins pushed a commit that referenced this pull request Mar 24, 2020
Co-Authored-By: Geoffrey Booth <GeoffreyBooth@users.noreply.github.com>
PR-URL: #32098
Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
MylesBorins pushed a commit to MylesBorins/node that referenced this pull request Apr 3, 2020
Co-Authored-By: Geoffrey Booth <GeoffreyBooth@users.noreply.github.com>
PR-URL: nodejs#32098
Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
MylesBorins pushed a commit that referenced this pull request Apr 3, 2020
Backport-PR-URL: #32610
Co-Authored-By: Geoffrey Booth <GeoffreyBooth@users.noreply.github.com>
PR-URL: #32098
Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
@codebytere codebytere mentioned this pull request Apr 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
doc Issues and PRs related to the documentations. esm Issues and PRs related to the ECMAScript Modules implementation.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

package.exports: false is not working with module loader
7 participants