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: improve paragraph in esm.md #34064

Merged
merged 0 commits into from
Jun 29, 2020
Merged

doc: improve paragraph in esm.md #34064

merged 0 commits into from
Jun 29, 2020

Conversation

Trott
Copy link
Member

@Trott Trott commented Jun 26, 2020

Edit for clarity, correct tense, and brevity.

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 doc Issues and PRs related to the documentations. esm Issues and PRs related to the ECMAScript Modules implementation. labels Jun 26, 2020
doc/api/esm.md Outdated
Comment on lines 126 to 132
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
until the next folder containing another `package.json`, are a
_package scope_. The `"type"` field defines how to treat `.js` files
within the package scope. Every package in a
project’s `node_modules` folder 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 contained `"type": "commonjs"`.
project’s dependencies have their own package scopes. If a `package.json` file
does not have a `"type"` field, the default `"type"` is `"commonjs"`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason to not go the full 80 characters per line?

Copy link
Member Author

Choose a reason for hiding this comment

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

Just to keep the diff clear. I can re-wrap to fill out the lines if that's preferred.

doc/api/esm.md Outdated
project’s dependencies have their own package scopes. A `package.json` lacking a
`"type"` field is treated as if it contained `"type": "commonjs"`.
project’s dependencies have their own package scopes. If a `package.json` file
does not have a `"type"` field, the default `"type"` is `"commonjs"`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it worth noting that invalid values are also interpreted as "commonjs"?

Suggested change
does not have a `"type"` field, the default `"type"` is `"commonjs"`.
does not have a `"type"` field or an invalid one, the default `"type"` is
`"commonjs"`.

Copy link
Member Author

Choose a reason for hiding this comment

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

If that's the case, then that seems like a good addition. I'd prefer that be a different pull request so that this one is kept purely style and grammar.

@Trott Trott closed this Jun 29, 2020
@Trott Trott merged commit 95eecd5 into nodejs:master Jun 29, 2020
@Trott
Copy link
Member Author

Trott commented Jun 29, 2020

Landed in 95eecd5

@Trott Trott deleted the tidy-esm branch June 29, 2020 03:27
MylesBorins pushed a commit that referenced this pull request Jul 14, 2020
Edit for clarity, correct tense, and brevity.

PR-URL: #34064
Reviewed-By: Guy Bedford <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Jul 14, 2020
MylesBorins pushed a commit that referenced this pull request Jul 16, 2020
Edit for clarity, correct tense, and brevity.

PR-URL: #34064
Reviewed-By: Guy Bedford <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
addaleax pushed a commit that referenced this pull request Sep 22, 2020
Edit for clarity, correct tense, and brevity.

PR-URL: #34064
Reviewed-By: Guy Bedford <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@codebytere codebytere mentioned this pull request Sep 28, 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.

6 participants