invertase / react-native-firebase

🔥 A well-tested feature-rich modular Firebase implementation for React Native. Supports both iOS & Android platforms for all Firebase services.
https://rnfirebase.io
Other
11.72k stars 2.22k forks source link

fix(analytics): update superstruct dependency #8153

Closed exzos28 closed 2 days ago

exzos28 commented 2 days ago

Related issues

Checklist

vercel[bot] commented 2 days ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-native-firebase ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 20, 2024 7:15pm
exzos28 commented 2 days ago

Could you restart "Testing E2E Android" if necessary? It looks like the emulator did not start for some reason.

mikehardy commented 2 days ago

android CI failure was unrelated, re-running it if the iOS runs completed I think this is good to go Fantastic! This one has personally bothered me for a long time as I'm the one usually updating dependencies and I hated seeing it stuck. Much appreciated.

RohovDmytro commented 19 hours ago

I do have an error while running the app, after resetting node_modules, yarn.lock and rebuilding the app from fresh.

 ERROR  TypeError: 0, _superstruct.define is not a function (it is undefined), js engine: hermes [Component Stack]

with

@react-native-firebase/analytics@npm:21.6.0
mikehardy commented 18 hours ago

rebuilding the app from fresh

@RohovDmytro 👋 looking in to this, what exact command are you running to rebuild the app from fresh?

mikehardy commented 18 hours ago

When I last ran this, this afternoon, it was already using 21.6.0 and did not have a build failure:

https://github.com/mikehardy/rnfbdemo/blob/main/make-demo.sh

RohovDmytro commented 18 hours ago

rebuilding the app from fresh

@RohovDmytro 👋 looking in to this, what exact command are you running to rebuild the app from fresh?

Resetting watchman cache, deleting node modules, yarn lock, running yarn install and then expo prebuild, and starting metro server with a reset config. I can provide the exact command line combo a bit later, if that would be important.

RohovDmytro commented 18 hours ago

The app does build, but crashes in production and yields the error above during development.

mikehardy commented 18 hours ago

I wonder fi you have the opposite problem of the issue that prompted the PR here, #5146 That is, perhaps some other module of yours requires the old superstruct but now we rely on the new one? Or perhaps there is some other expo-specific difference?

The app my script builds run fine in release and debug from completely clean setups (similar to what your commands do) - that's the whole point of that repro script really - to start from completely clean, do a reproducible series of steps, and have a full react-native-firebase integration run in debug and release modes. And it's working for me 🤔

RohovDmytro commented 10 hours ago

That is, perhaps some other module of yours requires the old superstruct but now we rely on the new one?

supestruct is used only via /analytics package.

├─ @react-native-firebase/analytics@npm:21.6.0
│  └─ superstruct@npm:2.0.2 (via npm:^2.0.2)
│
└─ @react-native-firebase/analytics@npm:21.6.0 [6c1df]
   └─ superstruct@npm:2.0.2 (via npm:^2.0.2)

That was the main reason I've assumed it would be enough to quickly blame open source and get away with commenting in PR discussion :)

Through years I've accumulated many ways of resetting caches. Let me try the bigest-badast--reset, maybe that will help :)

Either way, I would provide more actionable details or fix the issue my myself. Thanks for taking a look into it, @mikehardy.

exzos28 commented 10 hours ago

@RohovDmytro Did you update @react-native-firebase/analytics and @react-native-firebase/app as well? Maybe attach your yarn.lock, I'll take a look.

RohovDmytro commented 10 hours ago

Biggest-baddest-reset did not work. My next assumptions:

@exzos28, yes. All of them.

The amount of my dependencies in the current project made me mentally unstable. Please, be careful when looking at it. Thanks.

yarn.lock.zip

RohovDmytro commented 10 hours ago

Downgrading to @21.5.0 makes the project work (event without rebuilding, wow!). And that works for me :)

Another assumption: do we have latest and greatest code under 21.6.0 tag?

I have to move on with my sprint and will stop pretending to be helpful. The final straw from me is to rename the project current repository into @react-native-firearse/*, but that's also might be as helpful as previous comments.

I will post more if I'll find more.

exzos28 commented 10 hours ago

Interesting point - I also have react-native 75 and an older architecture. And only updating superstruct allowed to use analytics package.

mikehardy commented 2 hours ago

I looked through your yarn.lock @RohovDmytro and it all looks normal, unfortunately nothing jumps out at me there My reproductions of success are

https://github.com/invertase/react-native-firebase/blob/0103e12714de3c106128859eeadaf0fe07f9674c/tests/package.json#L32

And my make-demo.sh script which is the newest everything - rn0.76.2 + new architecture

and they both work - I just can't trigger this, and we haven't had others mention it yet either, so I suspect this is local somehow but I know what you mean by all the react-native project cleaning stuff (I've had PRs merged into react-native-clean-project - it's a constant battle). So it seems there may be something to this.

Unfortunately without a reproduction I can't move further with it, but I'll be listening in case something is posted