typings: add typing for string decoder by Ayase-252 · Pull Request #38229 · nodejs/node

Ayase-252 · 2021-04-13T15:40:00Z

This PR adds some JSDoc typings for internal/string_decoder.

Screenshots from my VS code:

normalizeEncoding

StringDecoder

StringDecoder.prototype.write

StringDecoder.prototype.end

StringDecoder.prototype.text

Ayase-252

I know the line violates the maximum line length, but I couldn’t come up with a good idea to fix it. 🤔

targos · 2021-04-14T05:56:08Z

+/**
+ *
+ * @param {string} enc
+ * @returns {"utf8" | "utf16le" | "hex" | "ascii" | "base64" | "latin1" | "base64url"}


Suggested change
* @returns{"utf8"|"utf16le"|"hex"|"ascii"|"base64"|"latin1"|"base64url"}
* @returns{"utf8"|"utf16le"|"hex"|"ascii"
*|"base64"|"latin1"|"base64url"}
What about this?

Thanks, it works

Co-authored-by: Michaël Zasso <targos@protonmail.com>

marsonya

I would suggest adding function descriptions in the jsdoc comments.

marsonya · 2021-04-19T18:50:54Z

 };

+/**
+ *


Suggested change
*
*Returnsanyremaininginputstoredintheinternalbufferasastring.
*Afterend()iscalled,thestringDecoderobjectcanbereusedfornewinput.

marsonya · 2021-04-19T18:53:18Z

 // buffers into a series of JS strings without breaking apart multi-byte
 // characters.
+/**
+ *


Move the above description inside the comment?

marsonya · 2021-04-19T18:54:27Z

 }

+/**
+ *


Suggested change
*
*Returnsadecodedstring,omittinganyincompletemultibyte
*charactersattheendoftheBuffer,orTypedArray,orDataView

Co-authored-by: marsonya <akhil.marsonya27@gmail.com>

marsonya

Mark optional params as such and assign default value using JSDoc conventions.

Kindly ignore it incase this is too much detailing. I know we don't want to duplicate the documentation here using JSDoc. I just feel that it would be important information for core developers to know whether a param is optional or not and what it's default value would be.

Ayase-252 · 2021-04-21T02:31:37Z

Good idea, I will address optional arugment issue

Co-authored-by: Akhil Marsonya <akhil.marsonya27@gmail.com>

aduh95

RSLGTM

nodejs-github-bot · 2021-12-30T23:26:48Z

Commit Queue failed

- Loading data for nodejs/node/pull/38229
✔ Done loading data for nodejs/node/pull/38229
----------------------------------- PR info ------------------------------------
Title typings: add typing for string decoder (#38229)
Author Qingyu Deng (@Ayase-252)
Branch Ayase-252:feature/typing-lib-internal-cipher -> nodejs:master
Labels string_decoder, typings, commit-queue-squash
Commits 4
- typings: add typing for internal/string_decoder
- fixup: typings: add typing for internal/string_decoder
- fixup: add function description
- fixup: mark optional parameters
Committers 1
- Qingyu Deng PR-URL: https://github.com/nodejs/node/pull/38229
Reviewed-By: James M Snell Reviewed-By: Antoine du Hamel ------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/38229
Reviewed-By: James M Snell Reviewed-By: Antoine du Hamel --------------------------------------------------------------------------------
ℹ This PR was created on Tue, 13 Apr 2021 15:40:00 GMT
✔ Approvals: 2
✔ - James M Snell (@jasnell) (TSC): https://github.com/nodejs/node/pull/38229#pullrequestreview-640365320
✔ - Antoine du Hamel (@aduh95) (TSC): https://github.com/nodejs/node/pull/38229#pullrequestreview-842115810
✔ Last GitHub CI successful
✖ No Jenkins CI runs detected
--------------------------------------------------------------------------------
✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/1639087835

aduh95 · 2021-12-30T23:44:48Z

Landed in a100a93

PR-URL: #38229 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

typings: add typing for internal/string_decoder
cd2230b

nodejs-github-bot added needs-ci PRs that need a full CI run. string_decoder Issues and PRs related to the string_decoder subsystem. labels Apr 13, 2021

Ayase-252 added the typings label Apr 13, 2021

Ayase-252 commented Apr 13, 2021
View reviewed changes

targos reviewed Apr 14, 2021
View reviewed changes

fixup: typings: add typing for internal/string_decoder
b9ccd90
Co-authored-by: Michaël Zasso <targos@protonmail.com>

marsonya requested changes Apr 19, 2021
View reviewed changes

Comment threadlib/string_decoder.js

marsonya reviewed Apr 19, 2021
View reviewed changes

Ayase-252 changed the title ~~typings: add typing for internal/string_decoder~~typings: add typing for string decoderApr 20, 2021

fixup: add function description
4aee5ee
Co-authored-by: marsonya <akhil.marsonya27@gmail.com>

jasnell approved these changes Apr 20, 2021
View reviewed changes

marsonya requested changes Apr 21, 2021
View reviewed changes

Comment threadlib/string_decoder.js Outdated
Comment threadlib/string_decoder.js Outdated

fixup: mark optional parameters
1d2dc41
Co-authored-by: Akhil Marsonya <akhil.marsonya27@gmail.com>

marsonya approved these changes Apr 21, 2021
View reviewed changes

Mesteery removed the needs-ci PRs that need a full CI run. label Oct 12, 2021

aduh95 approved these changes Dec 30, 2021
View reviewed changes

aduh95 added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Dec 30, 2021

nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 30, 2021

nodejs-github-bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Dec 30, 2021

aduh95 merged commit a100a93 into nodejs:masterDec 30, 2021

targos pushed a commit that referenced this pull request Jan 14, 2022
typings: add JSDoc for string_decoder
d733b56
PR-URL: #38229 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

targos mentioned this pull request Jan 16, 2022
v17.4.0 release proposal #41557
Merged

danielleadams pushed a commit that referenced this pull request Jan 31, 2022
typings: add JSDoc for string_decoder
1f7aa2b
PR-URL: #38229 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

danielleadams pushed a commit that referenced this pull request Feb 1, 2022
typings: add JSDoc for string_decoder
782dbbd
PR-URL: #38229 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>

danielleadams mentioned this pull request Feb 1, 2022
v16.14.0 proposal #41804
Merged

	* @returns{"utf8"\|"utf16le"\|"hex"\|"ascii"\|"base64"\|"latin1"\|"base64url"}
	* @returns{"utf8"\|"utf16le"\|"hex"\|"ascii"
	*\|"base64"\|"latin1"\|"base64url"}

 };
+/**
+ *

	*
	*Returnsanyremaininginputstoredintheinternalbufferasastring.
	*Afterend()iscalled,thestringDecoderobjectcanbereusedfornewinput.

 }
+/**
+ *

	*
	*Returnsadecodedstring,omittinganyincompletemultibyte
	*charactersattheendoftheBuffer,orTypedArray,orDataView

Uh oh!

Uh oh!

Conversation

Ayase-252 commented Apr 13, 2021

Uh oh!

Ayase-252 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

targosApr 14, 2021

Choose a reason for hiding this comment

Uh oh!

Ayase-252Apr 14, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

marsonya left a comment

Choose a reason for hiding this comment

Uh oh!

marsonyaApr 19, 2021

Choose a reason for hiding this comment

Uh oh!

marsonyaApr 19, 2021

Choose a reason for hiding this comment

Uh oh!

marsonyaApr 19, 2021

Choose a reason for hiding this comment

Uh oh!

marsonya left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Ayase-252 commented Apr 21, 2021

Uh oh!

aduh95 left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Dec 30, 2021

Uh oh!

aduh95 commented Dec 30, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

Ayase-252 left a comment •
edited
Loading