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: add package.json documentation #32970

Closed
wants to merge 1 commit into from

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Apr 21, 2020

Node.js has recently started to rely more on the package.json file, and I feel it would be helpful to have a separate document defining the fields we use (a bit like the guide on npm docs).

Currently, Conditional Exports is supported by both ESM and CJS loaders, and it is documented in the esm.md file, which arguably is confusing.

I have used package.json.md for the name of the file, maybe we could use something more broad like package_scope.md or packages.md so it might be useful for other things we want to document. Please weight in if you have thoughts.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • documentation is changed or added
  • commit message follows commit guidelines

@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to the documentations. label Apr 21, 2020
doc/api/esm.md Outdated Show resolved Hide resolved
Copy link
Member

@benjamingr benjamingr left a comment

Choose a reason for hiding this comment

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

Thank you for your contribution :) I like this change, I think it makes sense.

doc/api/package.json.md Outdated Show resolved Hide resolved
doc/api/package.json.md Outdated Show resolved Hide resolved
@MylesBorins MylesBorins added the esm Issues and PRs related to the ECMAScript Modules implementation. label Apr 21, 2020
@MylesBorins
Copy link
Contributor

@nodejs/modules this significantly refactors the documentation for ESM PTAL

Copy link
Member

@GeoffreyBooth GeoffreyBooth left a comment

Choose a reason for hiding this comment

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

I agree with the overall idea of this, I'm putting a block on it only to ensure this gets adequate review before this lands. This is a big change and I want to make sure we give it some thorough review.

doc/api/esm.md Outdated
### Package Scope and File Extensions

A folder containing a `package.json` file, and all subfolders below that folder
down until the next folder containing another `package.json`, is considered a
_package scope_. The `"type"` field defines how `.js` files should be treated
Copy link
Member

Choose a reason for hiding this comment

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

We still need this first sentence, that defines what a package scope is. I would perhaps keep this paragraph unchanged.

Copy link
Contributor

Choose a reason for hiding this comment

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

Does the whole section on package scope and file extension belong into the package.json section? It doesn't seem to be specific to ESM.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have added links to the definition of package scope in package.json.html, do you think that is sufficient?

Does the whole section on package scope and file extension belong into the package.json section? It doesn't seem to be specific to ESM.

It is actually a sub-section of Enabling, and because CJS is default in node, it makes sense to have it documented here IMO.

doc/api/index.md Outdated Show resolved Hide resolved
doc/api/package.json.md Outdated Show resolved Hide resolved
Comment on lines 10 to 11
_package scope_. This document aims to describe the fields used by Node.js
runtime.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
_package scope_. This document aims to describe the fields used by Node.js
runtime.
_package scope_. This document aims to describe the fields used by the
Node.js runtime.

Copy link
Member

Choose a reason for hiding this comment

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

The two sentences in this paragraph don't really belong together. The first defines a package scope, the second introduces this document.

The overall order of sections is a bit off, IMO. I would think that "name" should come first, and there should be an explanation of how this field is used with require and import. When one does require('foo'), does Node look for a folder named node_modules/foo or a package with "name": "foo"? What happens when they disagree? Etc. If Node doesn't use the field at all, we shouldn't be documenting the field.

After that I think should be introduction of package scope and "type". Then exports, conditional exports and finally "main".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The overall order of sections is a bit off, IMO.

I have chosen the alphabetical order to order the sections, but I reckon that's not the order most users (and even tools) do write package.json files. I can work on that, thank you for your suggestions.

there should be an explanation of how this field (Ed: "name") is used with require and import.

AFAIK Node uses this field only for self-referencing packages, as explained here: https://github.com/nodejs/node/pull/32970/files#diff-66d7676e29759d09461d2fac50fb2564R249-R250
In order to limit the risk of confusion, maybe could we add a link to the Resolution Algorithm – or move the doc section to this document, as it doesn't belong to ESM per se. What are your thoughts?

Copy link
Member

Choose a reason for hiding this comment

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

Perhaps in the introduction you can list all the fields that Node recognizes, in alphabetical order, with each field linked to its section? That way we can keep a section name like Conditional Exports, but in the introduction you can have something like “"exports": See Exports and Conditional Exports”. And then the sections can be in logical order of what makes the most sense for someone reading this document from top to bottom, but there’s still this table of contents to jump to a particular field.

As for the resolution algorithm, I think it differs in CommonJS versus ESM, so it's probably best kept in each of those sections and linked to from here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps in the introduction you can list all the fields that Node recognizes, in alphabetical order, with each field linked to its section?

Wouldn't that be redundant with the TOC?

Copy link
Member

Choose a reason for hiding this comment

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

Possibly, though there’s a disparity in the section names like Conditional Exports and the fields like "exports". We probably want to provide a way for people to look them up via either.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@GeoffreyBooth I've made some changes, now the TOC look like this:

* `package.json`
    * Introduction
    * Supported fields
        * `"name"`
        * `"exports"`
            * Subpath Exports
            * Package Exports Fallbacks
            * Exports Sugar
            * Conditional Exports
            * Nested conditions
        * `"main"`
        * `"type"`

Does that work for you?

Copy link
Member

@GeoffreyBooth GeoffreyBooth Apr 27, 2020

Choose a reason for hiding this comment

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

Sure, that looks great, except that I think you need "type" before "exports", because doesn't "exports" discuss topics relevant to that? The docs should make sense if someone were to read them from the top down.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe I'm missing something, but I don't see how "type" and "exports" are related... "type" is atm related to the "main" field, but #33086 should change that.

The way I see it, its sole purpose is to set the correct parser for .js files, which is different from other fields that affect the resolution algorithm, which is why I put it last.

doc/api/package.json.md Outdated Show resolved Hide resolved
doc/api/package.json.md Outdated Show resolved Hide resolved
doc/api/package.json.md Outdated Show resolved Hide resolved
@Trott
Copy link
Member

Trott commented Apr 27, 2020

@nodejs/documentation

@MylesBorins
Copy link
Contributor

@aduh95 I'm in the process of rewriting a chunk of the docs on exports in #33074

Just wanted to give you a heads up as it will likely create a conflict if mine lands first

@aduh95
Copy link
Contributor Author

aduh95 commented May 16, 2020

I've rebased to integrate the changes made in #33074 and #33220.

Thanks to everyone who has reviewed this PR and given feedback. Is there more I can do to have it ready to land? @GeoffreyBooth, would you say I have done a good enough job of addressing your comments?

Copy link
Member

@benjamingr benjamingr left a comment

Choose a reason for hiding this comment

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

Latest edition LGTM thanks for the improvements.

Copy link
Contributor

@guybedford guybedford left a comment

Choose a reason for hiding this comment

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

This seems like a nice refactoring to me, slowly coming around to it :) Thanks for your work and consistency pushing this forward @aduh95.


This field is ignored when the [`"exports"`][] field is provided. The `"main"`
field is supported in all versions of Node.js, but its capabilities are limited:
it only defines the main entry point of the package.
Copy link
Contributor

Choose a reason for hiding this comment

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

It can be worth noting here that main supports extension searching and does not require a leading ./ prefix, unlike "exports".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TIL you can feed an absolute path to "main" and Node.js doesn't complain and load the script even if it's in a completely unrelated directory 🤯

Also worth noting that "main" takes a path and "exports" file URLs. I wasn't sure how to phrase it, so I just added Its value is interpreted as a path.. I'm open to better suggestions x)

doc/api/package.json.md Outdated Show resolved Hide resolved
doc/api/package.json.md Outdated Show resolved Hide resolved
precedence in versions of Node.js that support `"exports"`.
[Conditional Exports][] can also be used within `"exports"` to define different
package entry points per environment, including whether the package is
referenced via `require` or via `import`.
Copy link
Contributor

Choose a reason for hiding this comment

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

It could be worth mentioning here that exports targets always start with ./ and that they must include the file extension.

Also, unlike the main, it only applies for package imports import 'pkg' and not when loading the package path as a relative or absolute specifier - import '../pkg' or import '/path/to/pkg.js.

Please don't quote the above verbatim but especially given recent usability reports and having this next to the main here, noting these differences is important to highlight.

Again, this is additive to the existing docs, so can be a separate PR too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! I have added a line to explicitly state that "exports" expects file URL starting with ./ and "main" expects a path. I also added that "exports" takes precedence over "main" only when referencing a package by its name. PTAL.

doc/api/package.json.md Outdated Show resolved Hide resolved
doc/api/esm.md Outdated
Comment on lines 81 to 83
Every package in a project’s `node_modules` folder usually contains its own
[`package.json`][] file, so each project’s dependencies have their own [package
scopes][]. A [`package.json`][] lacking a [`"type"`][] field is treated as if it
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Every package in a project’s `node_modules` folder usually contains its own
[`package.json`][] file, so each project’s dependencies have their own [package
scopes][]. A [`package.json`][] lacking a [`"type"`][] field is treated as if it
A folder containing a [`package.json`][] file, and all subfolders below that folder
down until the next folder containing another [`package.json`][], is considered a
_package scope_. The [`"type"`][] field defines how `.js` files should be treated
within a particular [`package.json`][] file’s package scope. Every package in a
project’s `node_modules` folder usually contains its own [`package.json`][] file,
so each project’s dependencies have their own [package scopes][]. A
[`package.json`][] lacking a [`"type"`][] field is treated as if it

It's a little weird to jump straight to discussing node_modules subfolders without first introducing the concept of package scope. The only difference between this version and the old version is that the first two sentences have been removed, but those are pretty vital sentences that define what a package scope is.

Also do we want to linkify every mention of package.json and "type" etc.?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's a little weird to jump straight to discussing node_modules subfolders without first introducing the concept of package scope. The only difference between this version and the old version is that the first two sentences have been removed, but those are pretty vital sentences that define what a package scope is.

Here are my thoughts on this topic. I can see 3 ways for handling it:

  1. Move the sentences to package.json.md#Introduction, and have a link in esm.md#Package Scope and File Extensions to the former.
  2. Keep the sentences in esm.md#Package Scope and File Extensions and add link(s) in package.json when package scope is mentioned.
  3. Duplicate the sentences.

The only one that makes sense to me is 1., and that's the one I have implemented in this PR. I have in mind that duplication is never a good thing and package scope doesn't belong to ESM. That being said, I'm open to being persuade otherwise.

Also do we want to linkify every mention of package.json and "type" etc.?

It was simply easier for me to do a search and replace, and it seemed tedious to me to decide which mention deserves a link, and which doesn't.

doc/api/esm.md Outdated Show resolved Hide resolved
doc/api/package.json.md Outdated Show resolved Hide resolved
<!-- YAML
added:
- v13.1.0
- v12.16.0
Copy link
Member

Choose a reason for hiding this comment

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

This could be confusing since "name" is a very old field, since the 0.x days of Node; I think it's only that Node (as opposed to npm) started using it fairly recently.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was added based on a user request (#33143). The introduction explicitly state that the documented fields are the one used by Node.js only. Do you suggest to just remove it or to make it clearer that it can be used alongside older versions of Node.js?

doc/api/package.json.md Outdated Show resolved Hide resolved
doc/api/package.json.md Outdated Show resolved Hide resolved
@GeoffreyBooth
Copy link
Member

Rereading all of this again, I'm struck by how much content really should be shared between docs pages. For example anyone reading the ECMAScript Modules page of the docs really should get a section on Conditional Exports, as most ESM package authors will want to know about that, but this PR moves that content into the package.json page of the docs—which makes sense in its own way, since that content isn't specific to ECMAScript Modules but rather applies to both CommonJS and ECMAScript Modules.

This is making me think that maybe the best organizational structure isn't three separate pages—(CommonJS) Modules, ECMAScript Modules and package.json—but rather just one page, Modules, that includes all this content. What does everyone else think?

@aduh95
Copy link
Contributor Author

aduh95 commented May 17, 2020

New changes PTAL:

  • I've moved "Self-referencing a package using its name" from esm.md to package.json.md – it's not a ESM-specific feature.
  • I've merged esm.md#Packages section into esm.md#Enabling. Given the content of the section, I think it makes more sense like this.

On master, the esm.md TOC look like this:

  • Introduction
  • Enabling
    • package.json "type" field
    • Package Scope and File Extensions
    • --input-type flag
  • Packages
    • Package Entry Points
      • Main Entry Point Export
      • Subpath Exports
      • Package Exports Fallbacks
      • Exports Sugar
      • Conditional Exports
      • Nested conditions
      • Self-referencing a package using its name
    • Dual CommonJS/ES Module Packages
      • Dual Package Hazard
      • Writing Dual Packages While Avoiding or Minimizing Hazards
        • Approach 1: Use an ES Module Wrapper
        • Approach 2: Isolate State
  • import Specifiers
    • Terminology
      • data: Imports
  • ...

On this PR it looks like this:

  • Introduction
  • Enabling
    • Package Scope and File Extensions
    • --input-type flag
    • Package Entry Points
    • Dual CommonJS/ES Module Packages
      • Dual Package Hazard
      • Writing Dual Packages While Avoiding or Minimizing Hazards
        • Approach 1: Use an ES Module Wrapper
        • Approach 2: Isolate State
  • import Specifiers
    • Terminology
      • data: Imports
  • ...

@guybedford
Copy link
Contributor

I tend to agree with @GeoffreyBooth's points about the structure here.

Just wondering, could it be worth treating the package.json page more like a very quick reference / index, that then links to the specific sections in either modules.md or esm.md with the full information? Sort of like how the CLI flag section works.

Separating modules.md from esm.md does seem like a somewhat arbitrary distinction but I think that it is fine due to the history here.

@GeoffreyBooth
Copy link
Member

I've given this some thought, and I do think we should merge modules.md and esm.md. The package.json info can be on that merged page. I think the merged page would still have big “CommonJS Modules” and “ECMAScript Modules” sections, with lots of subsections moved from the current modules.md and esm.md respectively; but the idea is that all content that is not particular to either module system should be outside of those big system-specific sections. And there's a lot of such content! So much so that it's made me more sure that we should merge the pages.

For example, I assume the Source Maps API applies to both module systems, but currently it only lives in modules.md; and conversely Package Exports applies to both but currently only lives in esm.md. And so on. Below I've written a draft outline of the merged page, and you can see how many sections are common, outside of the scope of either module system. For each section I've noted which former file the section came from, or if it is new.

Modules

  • Introduction (new, explain that Node.js has two module systems)

    • CommonJS Modules (based on modules.md introduction)
    • ECMAScript Modules (based on esm.md introduction)
  • Using (based on esm.md section “Enabling”)

    • package.json "type" field
    • Package Scope and File Extensions
    • --input-type flag
  • CommonJS Modules (new; all subsections from modules.md)

    • The Module Wrapper
    • The Module Scope
      • __dirname
      • __filename
      • exports
      • module
      • require(id)
        • require.cache
        • require.extensions
        • require.main
        • require.resolve(request[, options])
          • require.resolve.paths(request)
    • The module Object
      • module.children
      • module.exports
        • exports shortcut
      • module.filename
      • module.id
      • module.loaded
      • module.parent
      • module.paths
      • module.require(id)
    • Accessing The Main Module
    • Core Modules
    • File Modules
      • require of Files with the .mjs Extension (from modules.md “Addenda: The .mjs extension”)
    • Folders as Modules
    • Loading from the Global Folders
    • Loading from node_modules Folders
    • Cycles
    • Caching
      • Module Caching Caveats
  • ECMAScript Modules (new; all subsections from esm.md)

    • import Specifiers
      • Terminology
        • data: Imports
    • import.meta
    • Differences Between ES Modules and CommonJS
      • Mandatory File Extensions
      • No NODE_PATH
      • No require, exports, module.exports, __filename, __dirname
      • No require.resolve
      • No require.extensions
      • No require.cache
      • URL-based Paths
    • Interoperability with CommonJS
      • require
      • import statements
      • import() expressions
    • CommonJS, JSON, and Native Modules
    • Builtin Modules
    • Experimental JSON Modules
    • Experimental Wasm Modules
    • Experimental Top-Level await
    • Experimental Loaders
      • Hooks
        • resolve Hook
        • getFormat Hook
        • getSource Hook
        • transformSource Hook
        • getGlobalPreloadCode Hook
        • dynamicInstantiate Hook
      • Examples
        • HTTPS Loader
        • Transpiler Loader
  • Packages (from esm.md)

    • The package.json File (new section introducing the file itself, with very brief subsections that are nothing more than links to the appropriate sections)
      • "exports"
      • "main"
      • "name"
      • "type"
    • Package Entry Points
      • Main Entry Point Export
      • Subpath Exports
      • Package Exports Fallbacks
      • Exports Sugar
      • Conditional Exports
      • Nested conditions
      • Self-referencing a package using its name
    • Dual CommonJS/ES Module Packages
  • Utility Methods (from modules.md “The Module Object”)

    • module.builtinModules
    • module.createRequire(filename)
    • module.createRequireFromPath(filename)
    • module.syncBuiltinESMExports())
  • Source Map V3 Support (from modules.md)

    • module.findSourceMap(path[, error])
    • Class: module.SourceMap
      • new SourceMap(payload)
      • sourceMap.payload
      • sourceMap.findEntry(lineNumber, columnNumber)
  • Resolution Algorithms (new)

    • CommonJS Resolution Algorithm (from modules.md section “All Together...”)
    • ECMAScript Modules Resolution Algorithm (from esm.md section “Resolution Algorithm”)
      • Features
      • Resolver Algorithm
      • Customizing ESM specifier resolution algorithm
  • Tips for Package Manager Authors (from modules.md “Package Manager Tips”)

For reference, here are the current full outlines of each page:

Modules
  • Accessing the main module
  • Addenda: Package Manager Tips
  • Addenda: The .mjs extension
  • All Together...
  • Caching
    • Module Caching Caveats
  • Core Modules
  • Cycles
  • File Modules
  • Folders as Modules
  • Loading from node_modules Folders
  • Loading from the global folders
  • The module wrapper
  • The module scope
    • __dirname
    • __filename
    • exports
    • module
    • require(id)
      • require.cache
      • require.extensions
      • require.main
      • require.resolve(request[, options])
        • require.resolve.paths(request)
  • The module Object
    • module.children
    • module.exports
      • exports shortcut
    • module.filename
    • module.id
    • module.loaded
    • module.parent
    • module.paths
    • module.require(id)
  • The Module Object
    • module.builtinModules
    • module.createRequire(filename)
    • module.createRequireFromPath(filename)
    • module.syncBuiltinESMExports()
  • Source Map V3 Support
    • module.findSourceMap(path[, error])
    • Class: module.SourceMap
      • new SourceMap(payload)
      • sourceMap.payload
      • sourceMap.findEntry(lineNumber, columnNumber)
ECMAScript Modules
  • Introduction
  • Enabling
    • package.json "type" field
    • Package Scope and File Extensions
    • --input-type flag
  • Packages
    • Package Entry Points
      • Main Entry Point Export
      • Subpath Exports
      • Package Exports Fallbacks
      • Exports Sugar
      • Conditional Exports
      • Nested conditions
      • Self-referencing a package using its name
    • Dual CommonJS/ES Module Packages
  • import Specifiers
    • Terminology
      • data: Imports
  • import.meta
  • Differences Between ES Modules and CommonJS
    • Mandatory file extensions
    • No NODE_PATH
    • No require, exports, module.exports, __filename, __dirname
    • No require.resolve
    • No require.extensions
    • No require.cache
    • URL-based paths
  • Interoperability with CommonJS
    • require
    • import statements
    • import() expressions
  • CommonJS, JSON, and Native Modules
  • Builtin modules
  • Experimental JSON Modules
  • Experimental Wasm Modules
  • Experimental Top-Level await
  • Experimental Loaders
    • Hooks
      • resolve hook
      • getFormat hook
      • getSource hook
      • transformSource hook
      • getGlobalPreloadCode hook
      • dynamicInstantiate hook
    • Examples
      • HTTPS loader
      • Transpiler loader
  • Resolution Algorithm
    • Features
    • Resolver Algorithm
    • Customizing ESM specifier resolution algorithm

@aduh95
Copy link
Contributor Author

aduh95 commented Aug 29, 2020

The changes in this PR has evolved in very different direction since it's been opened, I'm closing it to open a new one, so we can start the conversation fresh (see #34970).

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.