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

firestorter/web supports only cjs builds #184

Open
dearlordylord opened this issue Jul 5, 2023 · 4 comments
Open

firestorter/web supports only cjs builds #184

dearlordylord opened this issue Jul 5, 2023 · 4 comments

Comments

@dearlordylord
Copy link

dearlordylord commented Jul 5, 2023

Hi, I found a weird issue on esm project.

firestorter/web uses only cjs build of firestore it seems

as a result, in esm project firestore compares firestore with firestore and finds that firestore is not firestore because firestore import is esm2017 in our case; there are just two same definitions of firestore that doesn't match when firebase lib code compares them with instanceof and it causes it to throw Expected first argument to collection() to be a CollectionReference, a DocumentReference or FirebaseFirestore even when the FirebaseFirestore is passed properly

the issue is that it doesn't understand that FirebaseFirestore from cjs and FirebaseFirestore from esm2017 builds are the same thing (because of instanceof)

the specific example of this is collection: (path: string) => collection(firestore, path), in firestorter/web where firestore provided is an import from esm2017 build, and collection comes from cjs build, then it internally checks firestore with t instanceof hh

when I copy-paste the firestorter/web function into my app, it starts using node_modules/firebase/node_modules/@firebase/firestore/dist/index.esm2017.js again and definitions of class hh { matches.

References: hh/vh classes mentioned are minimized https://github.com/firebase/firebase-js-sdk/blob/aea4a4471306b089a02b207d2df285dfc71666c7/packages/firestore/src/api/database.ts#L91 (I debugged this in browser, therefore shortened names from dists of Firebase js SDK)

and the comparison takes place in https://github.com/firebase/firebase-js-sdk/blob/aea4a4471306b089a02b207d2df285dfc71666c7/packages/firestore/src/lite-api/reference.ts#L406 (don't mind 'lite-api', I think they extend it down the line, I checked specifically that it works with the full API and that I don't use /lite anywhere)

I think providing two builds for firestorter/web like you have provided for root firestorter could fix this; I may make a minimal reproduction at some point also, if needed

@IjzerenHein
Copy link
Owner

Hmm interesting, not sure what's going on here

Seems like some kind of bundling issue indeed.

Just curious, what if you were to copy the contents of web/index.ts into your own project and use that to make a context and initialize firestorter with it. Does that work?

@dearlordylord
Copy link
Author

dearlordylord commented Jul 10, 2023

@IjzerenHein Yes it does, I actually walk around it like this for now. My project uses the classes from Firebase esm2017 so after doing so it all matches well

@IjzerenHein
Copy link
Owner

Alright, well that’s good to hear. I don’t really know what to do with this issue at this time, since the problem is so intertwined with your bundler. What bundles/stack are you using? I think the only way I can help you, is when you provide a minimal reproducable repo that I can test with

@dearlordylord
Copy link
Author

Hey, just a notification that I'm still experiencing this issue and would like to provide you with minimal repro at some point. No time right now but I remember. Nx is what I use to build. The specific configuration uses nextjs underneath.

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

No branches or pull requests

2 participants