FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

doc: clarify extensionless esm by azz · Pull Request #30657 · nodejs/node · GitHub

/ node Public

doc: clarify extensionless esm - #30657

Closed
azz wants to merge 5 commits into
nodejs:masterfrom
azz:patch-2
ConversationCommits (5)ChecksFiles changed
Closed

doc: clarify extensionless esm#30657
azz wants to merge 5 commits into
nodejs:masterfrom
azz:patch-2

Conversation

azz commented Nov 26, 2019

Copy link
Copy Markdown
Contributor

Fixes #30655

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 added the doc Issues and PRs related to the documentations. label Nov 26, 2019
MylesBorins suggested changes Nov 27, 2019
View reviewed changes

MylesBorins left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I don't think we should be documenting using a non-standard flag here in the docs... but we could improve the documentation above to make the difference between extensionless file path (path has no extension) and extensionless specifiers (path has extension, specifier doesn't, node resolves extension) clearer

Comment thread doc/api/esm.md Outdated
import './startup.js'; // Loaded as ES module because of package.json

// Extensionless. Requires 'node --es-module-specifier-resolution=node'
import './startup'; // Loaded as ES module because of package.json

MylesBorins Nov 27, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Extensionless files, as documented above, are different from extensionless specifiers (which you are documenting here). Extensionless files mean the file path itself has no extension e.g. /path/to/file, not the specifier e.g. './file' where path is /path/to/file.js.

azz Nov 27, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Ah I see. Yeah it could definitely be clarified.

MylesBorins Nov 28, 2019

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Would you be ok removing this line and instead clarifying above?

azz Dec 3, 2019

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Done. Had trouble writing it succinctly so let me know if it makes sense.

MylesBorins added the esm Issues and PRs related to the ECMAScript Modules implementation. label Nov 27, 2019

azz commented Nov 27, 2019

Copy link
Copy Markdown
Contributor Author

I don't think we should be documenting using a non-standard flag here

What do you mean by "non-standard flag"?

MylesBorins commented Nov 27, 2019

Copy link
Copy Markdown
Contributor

@azz it is an experimental flag, not an agreed upon official feature. Not having it labelled experimental was actually an oversight.

I'm actually working on making that clearer in #30678

azz changed the title doc: clarify required flag for extensionless esm doc: clarify extensionless esm Dec 3, 2019
MylesBorins approved these changes Dec 4, 2019
View reviewed changes

MylesBorins left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Took a quick stab at some slightly different text, unsure if it is better. Happy to see this land with the suggested changes ignore or landed.

Comment thread doc/api/esm.md Outdated
Comment thread doc/api/esm.md Outdated
azz and others added 2 commits December 5, 2019 18:03
doc: reword extensionless definition
bb49307 
Co-Authored-By: Myles Borins <myles.borins@gmail.com>
doc: reword clarification
e05c651 
Co-Authored-By: Myles Borins <myles.borins@gmail.com>
MylesBorins reviewed Dec 16, 2019
View reviewed changes
Comment thread doc/api/esm.md Outdated
doc: remove "note that"
ff75ccb 
Co-Authored-By: Myles Borins <myles.borins@gmail.com>

juanarbol commented Dec 18, 2019

Copy link
Copy Markdown
Member

I don't know if is necessary, but in #30699, CJS module will give suggestions when file extension is not provided, maybe it would be nice in this docs.

MylesBorins pushed a commit that referenced this pull request Dec 18, 2019
doc: clarify required flag for extensionless esm
3e5967b 
Fixes: #30655

PR-URL: #30657
Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>

MylesBorins commented Dec 18, 2019

Copy link
Copy Markdown
Contributor

Landed in 3e5967b

@juanarbol I think documenting #30699 is a great idea, but should likely happen in that PR (so we don't land the docs before the change is landed)

BridgeAR pushed a commit that referenced this pull request Jan 3, 2020
doc: clarify required flag for extensionless esm
a386114 
Fixes: #30655

PR-URL: #30657
Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
BridgeAR mentioned this pull request Jan 7, 2020
Merged
MylesBorins pushed a commit that referenced this pull request Jan 12, 2020
doc: clarify required flag for extensionless esm
c6b6e3f 
Fixes: #30655

PR-URL: #30657
Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
targos mentioned this pull request Jan 15, 2020
Closed
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
doc: clarify required flag for extensionless esm
2cbb358 
Fixes: #30655

PR-URL: #30657
Reviewed-By: Yorkie Liu <yorkiefixer@gmail.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
MylesBorins mentioned this pull request Feb 8, 2020
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers
MylesBorins MylesBorins approved these changes
yorkie yorkie approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

doc Issues and PRs related to the documentations. esm Issues and PRs related to the ECMAScript Modules implementation.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

doc: confusing support for extensionless ESM

5 participants

Back | FazBrowse Home | New Git URL