Skip to content

Fix missed CDP references when a string literal is followed by a comma - #24

Open
kirawth wants to merge 1 commit into
react:mainfrom
kirawth:fix-string-literal-references
Open

kirawth wants to merge 1 commit into
react:mainfrom
kirawth:fix-string-literal-references

Conversation

@kirawth

@kirawth kirawth commented Nov 2, 2026

Copy link
Copy Markdown

Summary

findAndPushMatches wraps each needle in (\b|(?<=\s|[()])) … (\b|(?=\s|[()])). String-literal needles like "Network.dataReceived" start and end with a quote, which is not a word character, so a match needs whitespace or a parenthesis right after the closing quote.

React Native emits events as cdp::jsonNotification("Network.dataReceived", params). A comma follows the quote, so these references were never found:

0  cdp::jsonNotification("Network.dataReceived", params.toDynamic()));
1  if (method == "Network.dataReceived") {
0  {"Network.dataReceived"}

HermesLegacyImplementationModel used plain \b"…"\b. That only matches when a word character touches both quotes, so its string-literal needle effectively never matched.

Changes

  • Add data/needleRegExp.tsx with buildNeedleRegExp(needles). It applies \b only at needle edges that are word characters. Quoted needles are already delimited by their quotes.
  • Use it in all three implementation models and remove their duplicated escapeRegExp.
  • Add data/needleRegExp.test.tsx.

Matching identifiers is unchanged: \b is still applied to both edges.

Impact

Running old vs. new against packages/react-native/ReactCommon/jsinspector-modern at current main:

references found distinct CDP names found
before 204 57 / 67
after 353 67 / 67

Test plan

  • New unit tests cover string literals followed by , ; : ) }, rejection of longer literals, whole-word identifier matching, and multiple needles.

findAndPushMatches wrapped each needle in word-boundary groups that only
accepted whitespace or a parenthesis after the match. String-literal
needles such as "Network.dataReceived" start and end with a quote, so a
call like jsonNotification("Network.dataReceived", params) never matched.
The legacy Hermes model used plain \b, which can only match a quoted
needle when a word character touches both quotes.

Move the regex into a shared buildNeedleRegExp helper that applies \b
only at needle edges that are word characters, and add tests.

Against jsinspector-modern this finds all 67 CDP names referenced as
string literals (previously 57), and 353 references instead of 204.
@meta-cla

meta-cla Bot commented Nov 2, 2026

Copy link
Copy Markdown

Hi @kirawth!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant