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

Jump to bottom

doc: importable node protocol URLs #35434

Closed
wants to merge 2 commits into from
Closed

doc: importable node protocol URLs #35434

wants to merge 2 commits into from

Conversation

bmeck
Copy link
Member

@bmeck bmeck commented Sep 30, 2020

Checklist

Documentation fixes nodejs/modules#555

CC: @nodejs/modules-active-members

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Sep 30, 2020 •
edited by bmeck

Review requested:

  • @nodejs/modules

@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 Sep 30, 2020
MylesBorins
MylesBorins approved these changes Sep 30, 2020
View reviewed changes
Copy link
Member

@MylesBorins MylesBorins left a comment

Choose a reason for hiding this comment

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

LGTM!

@MylesBorins
Copy link
Member

I'm thinking we should register this protocol with IANA. Any concerns?

mcollina
mcollina approved these changes Oct 1, 2020
View reviewed changes
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@bmeck
Copy link
Member Author

bmeck commented Oct 1, 2020

@MylesBorins seems apt

@bmeck
Copy link
Member Author

bmeck commented Oct 1, 2020

interestingly in order to register the scheme we must support # fragments (even if they point to the same resource)

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/33328/

@bmeck
Copy link
Member Author

bmeck commented Oct 2, 2020

I'm going to merge this today unless there are objections. We have talked about this in the past meetings so I don't think there is need for anything else.

@ExE-Boss
Copy link
Contributor

ExE-Boss commented Oct 2, 2020

interestingly in order to register the scheme we must support # fragments (even if they point to the same resource)

It’s also apparently necessary to support the ? query part. [RFC7595]

@bmeck
Copy link
Member Author

bmeck commented Oct 2, 2020

@ExE-Boss that is only for hierarchical URL schemes, which Node wouldn't be using.

@bmeck
Copy link
Member Author

bmeck commented Oct 2, 2020

We could also just note that in general matching new URL('node:fs?foo#bar') is the route i'd like to standardize as it is the least friction. Non-heirarchical, supports ?, and #. We can make ? and # fail to load still, they just have to parse. Wondering where we should open an issue on standardizing that into a formal document since IANA needs a permanent document. I'd avoid an internet draft if I could since this doesn't really affect internet infrastructure itself, but we do not have a formal place for archival storage in this organization that I know of.

bmeck added a commit that referenced this pull request Oct 2, 2020
@bmeck
doc: importable node protocol URLs
96a5200 
PR-URL: #35434
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
@bmeck
Copy link
Member Author

bmeck commented Oct 2, 2020

Landed in 96a5200

@bmeck bmeck closed this Oct 2, 2020
@bmeck bmeck mentioned this pull request Oct 2, 2020
Open
@ExE-Boss ExE-Boss mentioned this pull request Oct 5, 2020
Open
danielleadams pushed a commit that referenced this pull request Oct 6, 2020
@bmeck @danielleadams
doc: importable node protocol URLs
35b62ef 
PR-URL: #35434
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
@danielleadams danielleadams mentioned this pull request Oct 6, 2020
Merged
MylesBorins pushed a commit that referenced this pull request Nov 3, 2020
@bmeck @MylesBorins
doc: importable node protocol URLs
4368cd4 
PR-URL: #35434
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Nov 3, 2020
Merged
MylesBorins pushed a commit that referenced this pull request Nov 16, 2020
@bmeck @MylesBorins
doc: importable node protocol URLs
42c0dfc 
PR-URL: #35434
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
joesepi pushed a commit to joesepi/node that referenced this pull request Jan 8, 2021
@bmeck @joesepi
doc: importable node protocol URLs
e0042f4 
PR-URL: nodejs#35434
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
@bmeck bmeck deleted the doc-node-import-urls branch February 3, 2022 21:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcollina mcollina mcollina approved these changes

@jasnell jasnell jasnell approved these changes

@MylesBorins MylesBorins MylesBorins approved these changes

@jkrems jkrems jkrems approved these changes

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.

Document node: scheme
7 participants
@bmeck @nodejs-github-bot @MylesBorins @ExE-Boss @mcollina @jasnell @jkrems