Skip to content
This repository has been archived by the owner on Sep 2, 2023. It is now read-only.

Refactor module documentation #539

Closed
aduh95 opened this issue Aug 1, 2020 · 5 comments
Closed

Refactor module documentation #539

aduh95 opened this issue Aug 1, 2020 · 5 comments

Comments

@aduh95
Copy link

aduh95 commented Aug 1, 2020

There are a few issues open from users complaining about the current state of the docs:

I have two open PRs (nodejs/node#32970 and nodejs/node#33512) on node's repo trying to find another angle, and I'd like to thank @guybedford, @GeoffreyBooth and others who reviewed them. However, both approaches have failed and a discussion have initiated about a third attempt.
I think Node.js would benefit from getting more feedback before committing to a structure for this part of the docs, and this WG felt the right place to do so.

Currently there are two pages that deal with modules:

However there is no apparent logic on where a user can find documentation about features which a neither CJS nor ESM specific (package.json fields, package authoring, SourceMap support, the module core module, …).

In nodejs/node#33512 (comment), it was suggested that we could use something like:

  • Modules: CommonJS modules
  • Modules: ECMAScript modules
  • Modules: Package authoring configuration Packages
  • Modules: module core module API (including Source map v3 docs)
  • Modules: Source map v3 support (or could be moved to Debugger page)

The Modules: prefix is there to make sure those entries will appear next to one another in the TOC. What are your thoughts?

EDIT: I've included the suggestions from this thread (there are 3 comments at the time of edit).

@DerekNonGeneric
Copy link
Contributor

Thank you for creating this summary! I agree that the modules documentation needs some love, but I'm currently not in favor of merging the CJS and ESM docs. One is an experimental API, the other one is stable, and both are quite meaty. Don't get me wrong, I think some sort of restructuring needs to happen, but merging those seems like a bad idea as of right now.

When I get a chance, I will try to determine what my opinion would be on the best course of action to take.

@GeoffreyBooth
Copy link
Member

I'm currently not in favor of merging the CJS and ESM docs

I think he's proposing four separate top-level pages, similar to what I had suggested in nodejs/node#33512 (comment). The only other part was that I suggested moving the source maps section onto the Debugger page or onto its own page, since it's not really part of modules.

@guybedford
Copy link
Contributor

I like the four page separation. Perhaps Package configuration over Package authoring since in theory package configuration applies to non-published packages too. The source maps documentation seems fine to go with the module core module page since the API is related to that.

@DerekNonGeneric
Copy link
Contributor

DerekNonGeneric commented Aug 15, 2020

I somehow missed notifications on this issue and didn't realize that there were alternatives being proposed. @aduh95 made the PR before I could dedicate any amount of time looking into what I think would be best. I still stand by nodejs/node#34748 (review), but also wouldn't consider it a blocker, so I've pre-approved that PR. I would wait on @guybedford and @GeoffreyBooth before landing though.

jasnell pushed a commit to nodejs/node that referenced this issue Aug 19, 2020
The `module` core module is available for both CJS and ESM users, it
deserves its own page.

PR-URL: #34747
Refs: nodejs/modules#539
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
aduh95 added a commit to aduh95/node that referenced this issue Aug 19, 2020
Using a "Modules:" prefix groups all the related pages together when
using alphabetical order.

Refs: nodejs/modules#539
BethGriggs pushed a commit to nodejs/node that referenced this issue Aug 20, 2020
The `module` core module is available for both CJS and ESM users, it
deserves its own page.

PR-URL: #34747
Refs: nodejs/modules#539
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
BethGriggs pushed a commit to nodejs/node that referenced this issue Aug 20, 2020
The `module` core module is available for both CJS and ESM users, it
deserves its own page.

PR-URL: #34747
Refs: nodejs/modules#539
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Trott pushed a commit to nodejs/node that referenced this issue Aug 23, 2020
Using a "Modules:" prefix groups all the related pages together when
using alphabetical order.

Refs: nodejs/modules#539

PR-URL: #34663
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
BethGriggs pushed a commit to nodejs/node that referenced this issue Aug 24, 2020
Using a "Modules:" prefix groups all the related pages together when
using alphabetical order.

Refs: nodejs/modules#539

PR-URL: #34663
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
aduh95 added a commit to aduh95/node that referenced this issue Aug 26, 2020
This part of the docs aims to contain documentation regarding package
configuration that covers both ESM and CJS realms.

Refs: nodejs/modules#539
DerekNonGeneric pushed a commit to nodejs/node that referenced this issue Aug 29, 2020
This part of the docs aims to contain documentation regarding package
configuration that covers both ESM and CJS realms.

* Move Enabling section
* Update Enabling section
* Remove -u flag
* Package scopes do not carry through `node_modules` folders

Refs: nodejs/modules#539

Co-authored-by: Geoffrey Booth <webmaster@geoffreybooth.com>>
Co-authored-by: Guy Bedford <guybedford@gmail.com>

PR-URL: #34748
Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is>
Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Ruy Adorno <ruyadorno@github.com>
@aduh95
Copy link
Author

aduh95 commented Aug 29, 2020

Landed in core.

@aduh95 aduh95 closed this as completed Aug 29, 2020
MylesBorins pushed a commit to nodejs/node that referenced this issue Sep 24, 2020
This part of the docs aims to contain documentation regarding package
configuration that covers both ESM and CJS realms.

* Move Enabling section
* Update Enabling section
* Remove -u flag
* Package scopes do not carry through `node_modules` folders

Refs: nodejs/modules#539

Co-authored-by: Geoffrey Booth <webmaster@geoffreybooth.com>>
Co-authored-by: Guy Bedford <guybedford@gmail.com>

PR-URL: #34748
Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is>
Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Ruy Adorno <ruyadorno@github.com>
aduh95 added a commit to aduh95/node that referenced this issue Oct 23, 2020
Using a "Modules:" prefix groups all the related pages together when
using alphabetical order.

Refs: nodejs/modules#539

PR-URL: nodejs#34663
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
aduh95 added a commit to aduh95/node that referenced this issue Oct 23, 2020
This part of the docs aims to contain documentation regarding package
configuration that covers both ESM and CJS realms.

* Move Enabling section
* Update Enabling section
* Remove -u flag
* Package scopes do not carry through `node_modules` folders

Refs: nodejs/modules#539

Co-authored-by: Geoffrey Booth <webmaster@geoffreybooth.com>>
Co-authored-by: Guy Bedford <guybedford@gmail.com>

PR-URL: nodejs#34748
Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is>
Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Ruy Adorno <ruyadorno@github.com>
MylesBorins pushed a commit to nodejs/node that referenced this issue Nov 3, 2020
Using a "Modules:" prefix groups all the related pages together when
using alphabetical order.

Refs: nodejs/modules#539

Backport-PR-URL: #35757
PR-URL: #34663
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
MylesBorins pushed a commit to nodejs/node that referenced this issue Nov 3, 2020
This part of the docs aims to contain documentation regarding package
configuration that covers both ESM and CJS realms.

* Move Enabling section
* Update Enabling section
* Remove -u flag
* Package scopes do not carry through `node_modules` folders

Refs: nodejs/modules#539

Co-authored-by: Geoffrey Booth <webmaster@geoffreybooth.com>>
Co-authored-by: Guy Bedford <guybedford@gmail.com>

Backport-PR-URL: #35757
PR-URL: #34748
Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is>
Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Ruy Adorno <ruyadorno@github.com>
MylesBorins pushed a commit to nodejs/node that referenced this issue Nov 3, 2020
The `module` core module is available for both CJS and ESM users, it
deserves its own page.

PR-URL: #34747
Refs: nodejs/modules#539
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
MylesBorins pushed a commit to nodejs/node that referenced this issue Nov 16, 2020
Using a "Modules:" prefix groups all the related pages together when
using alphabetical order.

Refs: nodejs/modules#539

Backport-PR-URL: #35757
PR-URL: #34663
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
MylesBorins pushed a commit to nodejs/node that referenced this issue Nov 16, 2020
This part of the docs aims to contain documentation regarding package
configuration that covers both ESM and CJS realms.

* Move Enabling section
* Update Enabling section
* Remove -u flag
* Package scopes do not carry through `node_modules` folders

Refs: nodejs/modules#539

Co-authored-by: Geoffrey Booth <webmaster@geoffreybooth.com>>
Co-authored-by: Guy Bedford <guybedford@gmail.com>

Backport-PR-URL: #35757
PR-URL: #34748
Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is>
Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Ruy Adorno <ruyadorno@github.com>
MylesBorins pushed a commit to nodejs/node that referenced this issue Nov 16, 2020
The `module` core module is available for both CJS and ESM users, it
deserves its own page.

PR-URL: #34747
Refs: nodejs/modules#539
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants