fix(analytics): update superstruct dependency #8153

exzos28 · 2024-11-20T19:11:00Z

Related issues

Fixes [🐛] Incompatibility of superstruct versions when using @react-native-firebase/app and @solana/web3.js. #8152

Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
- Yes
My change supports the following platforms;
- Android
- iOS
My change includes tests;
- e2e tests added or updated in packages/\*\*/e2e
- jest tests added or updated in packages/\*\*/__tests__
I have updated TypeScript types that are affected by my change.
This is a breaking change;
- Yes
- No

vercel · 2024-11-20T19:11:06Z

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

mikehardy

Hey! You might have cracked this one.

I agree with the move to analytics
the new-style struct typing looks correct

Let's see what CI thinks...

exzos28 · 2024-11-20T19:43:46Z

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

mikehardy · 2024-11-20T19:58:05Z

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 · 2024-11-22T03:11:37Z

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 · 2024-11-22T03:50:12Z

rebuilding the app from fresh

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

mikehardy · 2024-11-22T03:51:28Z

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 · 2024-11-22T03:53:14Z

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 · 2024-11-22T03:55:08Z

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

mikehardy · 2024-11-22T04:17:25Z

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 · 2024-11-22T11:52:32Z

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 · 2024-11-22T11:57:16Z

@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 · 2024-11-22T12:00:30Z

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

I am using react-native 0.75.*
I am using hermes
Old arch

@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 · 2024-11-22T12:13:14Z

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 · 2024-11-22T12:23:23Z

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

mikehardy · 2024-11-22T19:58:45Z

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

the test app here with react-native 0.74.x + old architecture

react-native-firebase/tests/package.json

Line 32 in 0103e12

"react-native": "0.74.5",

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

LouisSyfer · 2024-11-26T14:29:16Z

Same issue with my new dev build. And with the last version of analytics.

mikehardy · 2024-11-26T14:32:00Z

@LouisSyfer please be specific, "same issue" doesn't mean much when attached to a merged PR (which is not an issue, and has lots of discussion on it, with no resolution)

LouisSyfer · 2024-11-26T15:04:39Z

Ok sorry @mikehardy !
I have a very similar problem as described below. The dev build failed when I want to test it with Expo Go. I just removed the last version of all firebase package I have installed (21.6.1), and reinstalled the 21.2.0 version. The build is ok now.
Sorry for the misanderstanding

exzos28 · 2024-11-26T15:07:57Z

Is it possible to use react-native-firebase/* with Expo Go? wow

exzos28 · 2024-11-26T15:11:37Z

@LouisSyfer If possible, please create an Issue with details and a reproducible repo and let's continue the discussion there.

mikehardy · 2024-11-26T15:58:54Z

Interesting.
In fact, it may be possible to use react-native-firebase in Expo Go with our new fallback support to firebase-js-sdk - at least on the web platform. Unsure on ios/android.
Historically there is no support for Expo Go though, as we use native modules not packed in the Expo Go app

LouisSyfer · 2024-11-26T19:24:43Z

You're right, it's weird 🧐
I'm not sure if you can read this :

Screen_Recording_20241126_200650_Boostarz.mp4

mikehardy · 2024-11-26T19:41:15Z

I think you're going to need expo-dev-client and get busy with prebuild

LouisSyfer · 2024-11-27T21:01:24Z

Expo-dev-client is installed.
I think maybe the latest versions of react-native-firebase are not compatible with SDK 52.

zallesov · 2024-11-28T15:39:36Z

Came here with the same issue.
_supersctruct.define is not a function

using version "21.6.1" for all firebase-related libs.

    "@react-native-firebase/analytics": "21.6.1",
    "@react-native-firebase/app": "^21.6.1",
    "@react-native-firebase/auth": "^21.6.1",
    "@react-native-firebase/crashlytics": "21.6.1",
    "@react-native-firebase/firestore": "21.6.1",
    "@react-native-firebase/functions": "21.6.1",
    "@react-native-firebase/perf": "21.6.1",
    "@react-native-firebase/storage": "21.6.1",

and

"react-native": "0.76",
"expo": "~52.0.11"

Superstruct is only used from analytics. No other dependency using it.

npm why superstruct
[email protected]
node_modules/superstruct
  superstruct@"^2.0.2" from @react-native-firebase/[email protected]
  node_modules/@react-native-firebase/analytics
    @react-native-firebase/analytics@"21.6.1" from the root project

Adding superstruct explicitly to dependencies does not make a difference.
Clean-ass build did not help too.
Not using Expo-Go

@mikehardy You suggested expo-dev-client. Can you elaborate how it can be used to debug or fix an issue?

mikehardy · 2024-11-28T15:51:34Z

You suggested expo-dev-client. Can you elaborate how it can be used to debug or fix an issue?

That was working off the hypothesis that this was happening in Expo Go, and I hoped that building natively / locally with a --prebuild might do the trick.

Apparently not, unfortunately.

RohovDmytro · 2024-11-28T16:03:00Z

Quick summary: define is undefined! :)

exzos28 · 2024-11-28T16:24:31Z

I tried to create an repro and I was unable to reproduce the error.
I just initialized an empty expo project and installed expo-dev-client.

I ran it on android.

Put console.log(define) inside node_modules/@react-native-firebase/analytics/lib/structs.js and it exists: [Function define].

Repository:
https://github.com/exzos28/rnfirebase-superstruct

You can try to reproduce the error.

exzos28 · 2024-11-28T16:40:12Z

@mikehardy I noticed that superstruct2.0 started using cjs modules, but Expo supports them (link).

exzos28 · 2024-11-28T16:45:11Z

Guys, could you run:

npx expo customize metro.config.js;
edit your metro.config.js like this:

const { getDefaultConfig } = require('expo/metro-config');
const config = getDefaultConfig(__dirname);
console.log(config.resolver.sourceExts)
module.exports = config;

execute npx expo;
and print here your logs.

P.S

You should see something like:

[
  'ts', 'tsx',
  'mjs', 'js',
  'jsx', 'json',
  'cjs', 'scss',
  'sass', 'css'
]

RohovDmytro · 2024-11-29T03:11:16Z

@exzos28 wow. You're awesome. Very nice catch!

Indeed, the problem, obviously, was in metro.config :)

For historical reasons I had this:

config.resolver.assetExts.push('cjs');
config.resolver.sourceExts.push('mjs');

Removing this lines fixed an issue for me.

And this was breaking superstruct. It is still strange as billion other deps are working fine, but anyway, thanks you for your idea!

exzos28 · 2024-11-29T09:48:09Z

@RohovDmytro that's strange.
I mean, you had already had mjs (and I hope cjs) in config.resolver.sourceExts.

Could you check, what is the result value contains in config.resolver.sourceExts after these pushes?

RohovDmytro · 2024-11-30T03:07:00Z

exzos28, sure.

Input:

config.resolver.assetExts.push('cjs');
config.resolver.sourceExts.push('mjs');

console.log(config.resolver.sourceExts);
console.log(config.resolver.assetExts);

Output:

[
  'ts',  'tsx',  'mjs',
  'js',  'jsx',  'json',
  'cjs', 'scss', 'sass',
  'css', 'mjs'
]
[
  'bmp',  'gif',  'jpg',  'jpeg',
  'png',  'psd',  'svg',  'webp',
  'm4v',  'mov',  'mp4',  'mpeg',
  'mpg',  'webm', 'aac',  'aiff',
  'caf',  'm4a',  'mp3',  'wav',
  'html', 'pdf',  'yaml', 'yml',
  'otf',  'ttf',  'zip',  'heic',
  'avif', 'db',   'cjs'
]

fix: update superstruct and move it to the only place it's used

f282d33

vercel bot deployed to Preview November 20, 2024 19:15 View deployment

mikehardy approved these changes Nov 20, 2024

View reviewed changes

github-actions bot added the Needs Attention label Nov 20, 2024

mikehardy changed the title ~~fix: update superstruct and move it to the only place it's used~~ fix(analytics): update superstruct dependency Nov 20, 2024

mikehardy added Workflow: Pending Merge Waiting on CI or similar and removed Needs Attention labels Nov 20, 2024

mikehardy merged commit 6db1fb4 into invertase:main Nov 20, 2024
17 of 18 checks passed

mikehardy removed the Workflow: Pending Merge Waiting on CI or similar label Nov 20, 2024

github-actions bot added the Needs Attention label Nov 22, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(analytics): update superstruct dependency #8153

fix(analytics): update superstruct dependency #8153

exzos28 commented Nov 20, 2024 •

edited by mikehardy

Loading

vercel bot commented Nov 20, 2024 •

edited

Loading

mikehardy left a comment

exzos28 commented Nov 20, 2024

mikehardy commented Nov 20, 2024

RohovDmytro commented Nov 22, 2024 •

edited

Loading

mikehardy commented Nov 22, 2024

mikehardy commented Nov 22, 2024 •

edited

Loading

RohovDmytro commented Nov 22, 2024

RohovDmytro commented Nov 22, 2024

mikehardy commented Nov 22, 2024

RohovDmytro commented Nov 22, 2024 •

edited

Loading

exzos28 commented Nov 22, 2024

RohovDmytro commented Nov 22, 2024 •

edited

Loading

RohovDmytro commented Nov 22, 2024 •

edited

Loading

exzos28 commented Nov 22, 2024

mikehardy commented Nov 22, 2024

LouisSyfer commented Nov 26, 2024

mikehardy commented Nov 26, 2024

LouisSyfer commented Nov 26, 2024

exzos28 commented Nov 26, 2024

exzos28 commented Nov 26, 2024

mikehardy commented Nov 26, 2024

LouisSyfer commented Nov 26, 2024

mikehardy commented Nov 26, 2024

LouisSyfer commented Nov 27, 2024

zallesov commented Nov 28, 2024

mikehardy commented Nov 28, 2024

RohovDmytro commented Nov 28, 2024 •

edited

Loading

exzos28 commented Nov 28, 2024

exzos28 commented Nov 28, 2024

exzos28 commented Nov 28, 2024 •

edited

Loading

RohovDmytro commented Nov 29, 2024

exzos28 commented Nov 29, 2024

RohovDmytro commented Nov 30, 2024 •

edited

Loading

fix(analytics): update superstruct dependency #8153

fix(analytics): update superstruct dependency #8153

Conversation

exzos28 commented Nov 20, 2024 • edited by mikehardy Loading

Related issues

Checklist

vercel bot commented Nov 20, 2024 • edited Loading

mikehardy left a comment

Choose a reason for hiding this comment

exzos28 commented Nov 20, 2024

mikehardy commented Nov 20, 2024

RohovDmytro commented Nov 22, 2024 • edited Loading

mikehardy commented Nov 22, 2024

mikehardy commented Nov 22, 2024 • edited Loading

RohovDmytro commented Nov 22, 2024

RohovDmytro commented Nov 22, 2024

mikehardy commented Nov 22, 2024

RohovDmytro commented Nov 22, 2024 • edited Loading

exzos28 commented Nov 22, 2024

RohovDmytro commented Nov 22, 2024 • edited Loading

RohovDmytro commented Nov 22, 2024 • edited Loading

exzos28 commented Nov 22, 2024

mikehardy commented Nov 22, 2024

LouisSyfer commented Nov 26, 2024

mikehardy commented Nov 26, 2024

LouisSyfer commented Nov 26, 2024

exzos28 commented Nov 26, 2024

exzos28 commented Nov 26, 2024

mikehardy commented Nov 26, 2024

LouisSyfer commented Nov 26, 2024

mikehardy commented Nov 26, 2024

LouisSyfer commented Nov 27, 2024

zallesov commented Nov 28, 2024

mikehardy commented Nov 28, 2024

RohovDmytro commented Nov 28, 2024 • edited Loading

exzos28 commented Nov 28, 2024

exzos28 commented Nov 28, 2024

exzos28 commented Nov 28, 2024 • edited Loading

RohovDmytro commented Nov 29, 2024

exzos28 commented Nov 29, 2024

RohovDmytro commented Nov 30, 2024 • edited Loading

exzos28 commented Nov 20, 2024 •

edited by mikehardy

Loading

vercel bot commented Nov 20, 2024 •

edited

Loading

RohovDmytro commented Nov 22, 2024 •

edited

Loading

mikehardy commented Nov 22, 2024 •

edited

Loading

RohovDmytro commented Nov 22, 2024 •

edited

Loading

RohovDmytro commented Nov 22, 2024 •

edited

Loading

RohovDmytro commented Nov 22, 2024 •

edited

Loading

RohovDmytro commented Nov 28, 2024 •

edited

Loading

exzos28 commented Nov 28, 2024 •

edited

Loading

RohovDmytro commented Nov 30, 2024 •

edited

Loading