GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — doc: clarify extensionless esm by azz · Pull Request #30657 · nodejs/node

Conversation

@azz

@azz 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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@azz
doc: clarify required flag for extensionless esm
e622474 
Fixes: nodejs#30655
@nodejs-github-bot nodejs-github-bot added the doc Issues and PRs related to Node.js documentation. label Nov 26, 2019
MylesBorins
MylesBorins suggested changes Nov 27, 2019
View reviewed changes

@MylesBorins MylesBorins left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
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 MylesBorins Nov 27, 2019

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@azz azz Nov 27, 2019

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ah I see. Yeah it could definitely be clarified.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@MylesBorins MylesBorins Nov 28, 2019

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@azz azz Dec 3, 2019

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@MylesBorins MylesBorins added the esm Issues and PRs related to the ECMAScript Modules implementation. label Nov 27, 2019
@azz

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"?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@MylesBorins

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
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@azz
doc: clarify extensionless file imports
26b4cb9
@azz azz changed the title doc: clarify required flag for extensionless esm doc: clarify extensionless esm Dec 3, 2019
MylesBorins
MylesBorins approved these changes Dec 4, 2019
View reviewed changes

@MylesBorins MylesBorins left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

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.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread doc/api/esm.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread doc/api/esm.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
azz and others added 2 commits December 5, 2019 18:03
@azz @MylesBorins
doc: reword extensionless definition
bb49307 
Co-Authored-By: Myles Borins <[email protected]>
@azz @MylesBorins
doc: reword clarification
e05c651 
Co-Authored-By: Myles Borins <[email protected]>
MylesBorins
MylesBorins reviewed Dec 16, 2019
View reviewed changes
Comment thread doc/api/esm.md Resolved

Uh oh!

There was an error while loading. Please reload this page.
@azz @MylesBorins
doc: remove "note that"
ff75ccb 
Co-Authored-By: Myles Borins <[email protected]>
@juanarbol

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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
MylesBorins pushed a commit that referenced this pull request Dec 18, 2019
@azz @MylesBorins
doc: clarify required flag for extensionless esm
3e5967b 
Fixes: #30655

PR-URL: #30657
Reviewed-By: Yorkie Liu <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
@MylesBorins

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)
juanarbol reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@MylesBorins MylesBorins closed this Dec 18, 2019
BridgeAR pushed a commit that referenced this pull request Jan 3, 2020
@azz @BridgeAR
doc: clarify required flag for extensionless esm
a386114 
Fixes: #30655

PR-URL: #30657
Reviewed-By: Yorkie Liu <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
@BridgeAR BridgeAR mentioned this pull request Jan 7, 2020
v13.6.0 proposal #31238
Merged
MylesBorins pushed a commit that referenced this pull request Jan 12, 2020
@azz @MylesBorins
doc: clarify required flag for extensionless esm
c6b6e3f 
Fixes: #30655

PR-URL: #30657
Reviewed-By: Yorkie Liu <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
@targos targos mentioned this pull request Jan 15, 2020
v12.15.0 release proposal #31368
Closed
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
@azz @BethGriggs
doc: clarify required flag for extensionless esm
2cbb358 
Fixes: #30655

PR-URL: #30657
Reviewed-By: Yorkie Liu <[email protected]>
Reviewed-By: Myles Borins <[email protected]>
@MylesBorins MylesBorins mentioned this pull request Feb 8, 2020
v12.16.0 proposal #31691
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@MylesBorins MylesBorins MylesBorins approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

doc Issues and PRs related to Node.js documentation. 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

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@azz @MylesBorins @juanarbol @nodejs-github-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.