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

refactor[react-devtools-shared]: minor parsing improvements and modifications #27661

Merged

Conversation

hoxyq
Copy link
Contributor

@hoxyq hoxyq commented Nov 7, 2023

Had these stashed for some time, it includes:

  • Some refactoring to remove unnecessary FlowFixMes and type castings via any.
  • Optimized version of parsing component names. We encode string names to utf8 and then pass it serialized from backend to frontend in a single array of numbers. Previously we would call slice to get the corresponding encoded string as a subarray and then parse each character. New implementation skips slice step and just receives left and right ranges for the string to parse.
  • Early break instead of continue when Store receives unexpected operation, like removing an element from the Store, which is not registered yet.

@hoxyq hoxyq requested a review from kassens November 7, 2023 15:11
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Nov 7, 2023
@hoxyq hoxyq force-pushed the devtools/minor-parsing-improvements-and-codemods branch from 7f4b770 to b81ba20 Compare November 7, 2023 15:17
Copy link
Member

@kassens kassens left a comment

Choose a reason for hiding this comment

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

Is the message format a JS arrray of JS numbers or a UintArray?

Especially, if it's already a UintArray it might be even better to explore the TextEncoder and TextDecoder APIs (https://developer.mozilla.org/en-US/docs/Web/API/TextDecoder) that can read/write directly into such an array.

@hoxyq
Copy link
Contributor Author

hoxyq commented Nov 7, 2023

Is the message format a JS arrray of JS numbers or a UintArray?

Especially, if it's already a UintArray it might be even better to explore the TextEncoder and TextDecoder APIs (https://developer.mozilla.org/en-US/docs/Web/API/TextDecoder) that can read/write directly into such an array.

It is a JS array of JS numbers. Strings encoding currently uses just charCodeAt:

export function utfEncodeString(string: string): Array<number> {
const cached = encodedStringCache.get(string);
if (cached !== undefined) {
return cached;
}
const encoded = [];
let i = 0;
let charCode;
while (i < string.length) {
charCode = string.charCodeAt(i);
// Handle multibyte unicode characters (like emoji).
if ((charCode & 0xf800) === 0xd800) {
encoded.push(surrogatePairToCodePoint(charCode, string.charCodeAt(++i)));
} else {
encoded.push(charCode);
}
++i;
}
encodedStringCache.set(string, encoded);
return encoded;
}

Encoded strings is not the only thing we pass like this, there are also other values, which also won't fit into Uint8.

@hoxyq hoxyq merged commit c897260 into facebook:main Nov 7, 2023
2 checks passed
@hoxyq hoxyq deleted the devtools/minor-parsing-improvements-and-codemods branch November 7, 2023 16:39
hoxyq added a commit that referenced this pull request Nov 29, 2023
### Breaking
* refactor[devtools]: highlight an array of elements for native
([hoxyq](https://github.com/hoxyq) in
[#27734](#27734))

### Features
* feat[devtools]: display Forget badge for the relevant components
([hoxyq](https://github.com/hoxyq) in
[#27709](#27709))

### Other
* Added windows powershell syntax to build scripts
([PrathamLalwani](https://github.com/PrathamLalwani) in
[#27692](#27692))
* refactor[react-devtools-shared]: minor parsing improvements and
modifications ([hoxyq](https://github.com/hoxyq) in
[#27661](#27661))
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
…ications (facebook#27661)

Had these stashed for some time, it includes:
- Some refactoring to remove unnecessary `FlowFixMe`s and type castings
via `any`.
- Optimized version of parsing component names. We encode string names
to utf8 and then pass it serialized from backend to frontend in a single
array of numbers. Previously we would call `slice` to get the
corresponding encoded string as a subarray and then parse each
character. New implementation skips `slice` step and just receives
`left` and `right` ranges for the string to parse.
- Early `break` instead of `continue` when Store receives unexpected
operation, like removing an element from the Store, which is not
registered yet.
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
### Breaking
* refactor[devtools]: highlight an array of elements for native
([hoxyq](https://github.com/hoxyq) in
[facebook#27734](facebook#27734))

### Features
* feat[devtools]: display Forget badge for the relevant components
([hoxyq](https://github.com/hoxyq) in
[facebook#27709](facebook#27709))

### Other
* Added windows powershell syntax to build scripts
([PrathamLalwani](https://github.com/PrathamLalwani) in
[facebook#27692](facebook#27692))
* refactor[react-devtools-shared]: minor parsing improvements and
modifications ([hoxyq](https://github.com/hoxyq) in
[facebook#27661](facebook#27661))
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
…ications (#27661)

Had these stashed for some time, it includes:
- Some refactoring to remove unnecessary `FlowFixMe`s and type castings
via `any`.
- Optimized version of parsing component names. We encode string names
to utf8 and then pass it serialized from backend to frontend in a single
array of numbers. Previously we would call `slice` to get the
corresponding encoded string as a subarray and then parse each
character. New implementation skips `slice` step and just receives
`left` and `right` ranges for the string to parse.
- Early `break` instead of `continue` when Store receives unexpected
operation, like removing an element from the Store, which is not
registered yet.

DiffTrain build for commit c897260.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants