Skip to content

fix: update phantom ref test from failing to passing#150

Merged
RyanCommits merged 8 commits into
fullstorydev:masterfrom
TaduJR:fix/react-19-phantom-ref-test-update
May 13, 2026
Merged

fix: update phantom ref test from failing to passing#150
RyanCommits merged 8 commits into
fullstorydev:masterfrom
TaduJR:fix/react-19-phantom-ref-test-update

Conversation

@TaduJR
Copy link
Copy Markdown
Contributor

@TaduJR TaduJR commented Apr 4, 2026

Summary

Updates the it.failing test added in #149 to it, since the phantom ref injection bug is now fixed by fullstorydev/fullstory-babel-plugin-react-native#64.

The non-enumerable synthetic ref fix in the babel plugin prevents sharedRefWrapper from leaking through {...rest} spreads in custom components, so the test now passes as expected.

Dependency

This PR should land after fullstorydev/fullstory-babel-plugin-react-native#64 is merged and a new version of @fullstory/babel-plugin-react-native is published.

@RyanCommits
Copy link
Copy Markdown
Contributor

@TaduJR Looks like there are lint errors https://app.circleci.com/pipelines/gh/fullstorydev/fullstory-react-native/468/workflows/625aef3c-8ade-410c-80f3-b766680ae718/jobs/526

Also, there will be two additional failing tests will your change from fullstorydev/fullstory-babel-plugin-react-native#64:
Reading FS properties on iOS › Reads fs properties correctly
and Reading FS properties on iOS › Annotate plugin annotates correctly

@TaduJR
Copy link
Copy Markdown
Contributor Author

TaduJR commented Apr 8, 2026

@RyanCommits,

The CI failure is expected this PR depends on fullstorydev/fullstory-babel-plugin-react-native#64 being merged and published first.

CI runs npm ci which installs the current published @fullstory/babel-plugin-react-native (unfixed), so the phantom ref test fails because the bug is still present. Once fullstorydev/fullstory-babel-plugin-react-native#64 is merged and a new version is published, CI will install the fixed plugin and all tests will pass.

I've verified locally:

Also fixed the lint formatting issue from the previous push.

@RyanCommits
Copy link
Copy Markdown
Contributor

Unfortunately this is not something that I am seeing locally when running npx jest --config jest.config.js --clearCache && npm run test:src. I am seeing two tests fail.

What we can do to verify in CI is change the npm install path to your branch:
"@fullstory/babel-plugin-react-native": "github:TaduJR/fullstory-babel-plugin-react-native#fix/react-19-non-enumerable-synthetic-refs".

You'll also need to change these lines to include src/index.js

@TaduJR
Copy link
Copy Markdown
Contributor Author

TaduJR commented Apr 9, 2026

Hi @RyanCommits,

As discussed, I've updated the dependency to install from my fix branch:

"@fullstory/babel-plugin-react-native": "github:TaduJR/fullstory-babel-plugin-react-native#fix/react-19-non-enumerable-synthetic-refs"

I also committed lib/index.js to the babel plugin branch so it's available when installed from GitHub (since devDependencies aren't installed for GitHub deps, the prepare script can't build).

The package.json dependency should be reverted back to a version range (e.g. "^1.7.0") before merging, once fullstorydev/fullstory-babel-plugin-react-native#64 is merged and published.

@TaduJR
Copy link
Copy Markdown
Contributor Author

TaduJR commented Apr 15, 2026

Hi @RyanCommits,

Thanks for merging fullstorydev/fullstory-babel-plugin-react-native#64

Bumped @fullstory/babel-plugin-react-native to ^1.6.3 to pick up the fix from fullstorydev/fullstory-babel-plugin-react-native#64. CI has passed now.

@TaduJR
Copy link
Copy Markdown
Contributor Author

TaduJR commented Apr 28, 2026

Hey @RyanCommits

How is it going on this one?

@TaduJR
Copy link
Copy Markdown
Contributor Author

TaduJR commented May 12, 2026

Seems @RyanCommits might be OoO

Anyone would like to review?

cc @JoshMiers-FS @martin-fs @ScottLNorvell

TIA 🙏.

Comment thread src/__tests__/index.test.tsx Outdated
@TaduJR TaduJR requested a review from RyanCommits May 13, 2026 08:20
Copy link
Copy Markdown
Contributor

@RyanCommits RyanCommits left a comment

Choose a reason for hiding this comment

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

LGTM, update to 1.6.4

Comment thread package.json Outdated
@TaduJR TaduJR requested a review from RyanCommits May 13, 2026 21:06
@RyanCommits RyanCommits merged commit 414e451 into fullstorydev:master May 13, 2026
2 checks passed
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.

2 participants