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: clarify escaping for ES modules #41074

Merged
merged 1 commit into from Dec 7, 2021
Merged

doc: clarify escaping for ES modules #41074

merged 1 commit into from Dec 7, 2021

Conversation

notroid5
Copy link
Contributor

@notroid5 notroid5 commented Dec 3, 2021

Change proposed according to #41052
This should make it clear how to escape special characters like #.
I don't know if there are any more, though.
Also, how you would call this in general?
I found it as "Javascript Escape Sequence", is this a known term?

(Also, the first link below contains a dot at the end:
https://github.com/nodejs/node/blob/HEAD/CONTRIBUTING.md.
That should probably be removed.)

Edit: Changed text slightly, so the next check should be successful.

@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 Dec 3, 2021
doc/api/esm.md Outdated
@@ -156,7 +156,7 @@ typically configured server.
### URLs

ES modules are resolved and cached as URLs. This means that files containing
special characters such as `#` and `?` need to be escaped.
special characters need to be replaced with escape sequences, such as `#` with `%23` and `?` with `%3F`.
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
special characters need to be replaced with escape sequences, such as `#` with `%23` and `?` with `%3F`.
special characters must be [percent-encoded][], such as `#` with `%23` and `?` with `%3F`.

Then add a new reference at the bottom of the esm.md to:

[percent-encoded]: url.md#percent-encoding-in-urls

Copy link
Member

Choose a reason for hiding this comment

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

Maybe either the bit being changed or the percent-encoding-in-urls section of url.md could point to encodeURIComponent()?

Copy link
Member

Choose a reason for hiding this comment

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

Maybe either the bit being changed or the percent-encoding-in-urls section of url.md could point to encodeURIComponent()?

Or https://developer.mozilla.org/en-US/docs/Glossary/percent-encoding?

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'd say the link to url.md#percent-encoding-in-urls suffices here.
The other links could then be referenced in url.md#percent-encoding-in-urls. (https://developer.mozilla.org/en-US/docs/Glossary/percent-encoding and https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/encodeURIComponent)

@ljharb
Copy link
Member

ljharb commented Dec 3, 2021

cc @nodejs/modules

doc/api/esm.md Outdated
Comment on lines 158 to 159
ES modules are resolved and cached as URLs. This means that files containing
special characters such as `#` and `?` need to be escaped.
Copy link
Member

@Trott Trott Dec 3, 2021

Choose a reason for hiding this comment

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

FWIW, the word escape does not appear in the relevant MDN doc, so maybe "encoded" is the way to go instead of "escaped" or "replaced with escape sequences". Also, and I know this wasn't introduced in this PR, but "files containing" makes it sound we're talking about the file contents when we're talking about the URL.

Suggested change
ES modules are resolved and cached as URLs. This means that files containing
special characters such as `#` and `?` need to be escaped.
ES modules are resolved and cached as URLs. This means that characters such as
`#` and `?` need to be [percent-encoded][].

Then add to the bottom:

[percent-encoding]: https://developer.mozilla.org/en-US/docs/Glossary/percent-encoding

@notroid5 notroid5 changed the title Clarify escaping for ES modules Doc: Clarify escaping for ES modules Dec 4, 2021
This should make it clear(er) how to escape special characters like `#`
and `?`.

Ref: #41052
@Trott Trott changed the title Doc: Clarify escaping for ES modules doc: clarify escaping for ES modules Dec 4, 2021
@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Dec 4, 2021
@aduh95 aduh95 added the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 7, 2021
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 7, 2021
@nodejs-github-bot nodejs-github-bot merged commit 3d5a7de into nodejs:master Dec 7, 2021
@nodejs-github-bot
Copy link
Collaborator

Landed in 3d5a7de

@notroid5 notroid5 deleted the patch-1 branch December 7, 2021 16:00
danielleadams pushed a commit that referenced this pull request Dec 13, 2021
This should make it clear(er) how to escape special characters like `#`
and `?`.

Ref: #41052

PR-URL: #41074
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
danielleadams pushed a commit that referenced this pull request Dec 14, 2021
This should make it clear(er) how to escape special characters like `#`
and `?`.

Ref: #41052

PR-URL: #41074
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
danielleadams pushed a commit that referenced this pull request Jan 31, 2022
This should make it clear(er) how to escape special characters like `#`
and `?`.

Ref: #41052

PR-URL: #41074
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
danielleadams pushed a commit that referenced this pull request Jan 31, 2022
This should make it clear(er) how to escape special characters like `#`
and `?`.

Ref: #41052

PR-URL: #41074
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Linkgoron pushed a commit to Linkgoron/node that referenced this pull request Jan 31, 2022
This should make it clear(er) how to escape special characters like `#`
and `?`.

Ref: nodejs#41052

PR-URL: nodejs#41074
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
danielleadams pushed a commit that referenced this pull request Feb 1, 2022
This should make it clear(er) how to escape special characters like `#`
and `?`.

Ref: #41052

PR-URL: #41074
Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@danielleadams danielleadams mentioned this pull request Feb 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. 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.

None yet

8 participants