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

Expo React Native does not work with mjs files #47

Closed
tobicrain opened this issue Oct 10, 2022 · 30 comments
Closed

Expo React Native does not work with mjs files #47

tobicrain opened this issue Oct 10, 2022 · 30 comments

Comments

@tobicrain
Copy link

By using the SDK with Expo (in my Pocketbase React SDK), Jest/Expo does not currently support MJS files. Maybe there is a way to replace the default entry ("main": "./dist/pocketbase.es.mjs") with something similar, also supported by Jest

@ganigeorgiev
Copy link
Member

ganigeorgiev commented Oct 10, 2022

This looks similar to #39.

The easiest option in my opinion will be to allow mjs files in your expo config as show in portabletext/react-portabletext#6 (comment).

@tobicrain
Copy link
Author

I mean, there are prettier things :D but if there's no other way, we'll probably leave it at that for now 👍🏻

@tobicrain
Copy link
Author

do you know if the same errors occur with react native without expo?

@ganigeorgiev
Copy link
Member

Sorry, I don't want to change the package.json entrypoint. The current package setup works with almost all common bundlers (including older webpack versions due to nuxt2) and the mjs extension is to ensure that the esm file will be loaded when the bundler doesn't take the module property in consideration.

It will be better to allow loading .mjs file anyway in your config, since I imagine most new packages will use it (it is also recommended by the V8 docs - https://v8.dev/features/modules#mjs).


do you know if the same errors occur with react native without expo?

I'm not sure. I only briefly tested a react native setup with https://snack.expo.dev/ and it works as expected there.
Another user recently reported an issue with react native related to the AuthStore (#42) but I wasn't able to reproduce it and currently I don't have a modern android device to test it properly (I have a very old galaxy s3 which I almost never use and it has android 4 or something like that, so its definitely not a reliable test device).

@tobicrain
Copy link
Author

Hey I tried to introduce the expo webpack configuration but it didn't fix my error

@ganigeorgiev
Copy link
Member

ganigeorgiev commented Oct 11, 2022

@tobicrain Could you post here the exact error message? This weekend I might try to find some time and investigate it further.

@ganigeorgiev ganigeorgiev reopened this Oct 11, 2022
@ganigeorgiev
Copy link
Member

ganigeorgiev commented Oct 11, 2022

@tobicrain In addition, you may want to check the gist linked in this issue - expo/expo#8809 (comment).

Furthermore, as suggested in expo/expo#15836, you could try adding mjs in your local metro.config.js.

@tobicrain
Copy link
Author

tobicrain commented Oct 11, 2022

@ganigeorgiev Unable to resolve "pocketbase" from "node_modules/pocketbase-react/lib/context/Pocketbase.js"

Bildschirmfoto 2022-10-11 um 14 19 54

My sdk - pocketbase-react, is using current pocketbase js version.
(yesterday I forked the js SDK and changed main entry to cjs temporary that everything works fine for me)

@ganigeorgiev
Copy link
Member

From the extensions list in the error message it doesn't seem that mjs is added there. How did you add it to your project?

Could you reproduce the same error in https://snack.expo.dev/ (for me it works fine there, for both the android and ios emulators)?

Maybe this is not an expo issue but a TypeScript one because by default TS also cannot load es modules (#34). In newer TS versions there is now nodenext compiler.module option - https://www.typescriptlang.org/docs/handbook/esm-node.html.

In any case, if you want to use the legacy cjs module, you can also try importing it directly like:

import PocketBase from 'pocketbase/cjs'

But I don't recommend the above - you should either use es modules and import style statements or cjs and require style statements.

@tobicrain
Copy link
Author

yeah I tried everything to avoid forking the sdk, but because I have to dynamically decide which import to use, I had to fork it

@ganigeorgiev
Copy link
Member

ganigeorgiev commented Oct 11, 2022

I don't understand what do you mean by "I have to dynamically decide which import to use". Providing always the cjs as main entrypoint will cause loading the cjs module in your case, aka. practically you are using import style statement with cjs module and this probably works only because you have a nodeResolver plugin enabled or something similar.

As mentioned earlier, registering the mjs in your bundler config should resolve the issue. Without any context it's really hard to reproduce it because there are a lot of dependencies involved (are you using babel, ts and/or other transpilers?).
Could you post what did you try to register the extension?

@tobicrain
Copy link
Author

tobicrain commented Oct 11, 2022

In any case, if you want to use the legacy cjs module, ... import PocketBase from 'pocketbase/cjs'

And that doesn't help me because I only want to import "pocketbase/cjs" if the sdk is used in react native, otherwise "pocketbase" is fine.

I'll try to recreate the whole thing with expo snack and send it to you

@tobicrain
Copy link
Author

tobicrain commented Oct 13, 2022

@ganigeorgiev
https://snack.expo.dev/@tkrainhoefner/happy-salsa
If you fill in line 8-13 in App.tsx you will see that Expo Snack is all working fine, but if you download it as a zip file and run it locally you will see the error I am getting

@ganigeorgiev
Copy link
Member

ganigeorgiev commented Oct 13, 2022

I'm not sure what the above package is supposed to test. I'm not able to run it locally but that's because there seems to be some dependency misconfiguration (I'm not sure how it works in expo snack):

[gani@dev happy-salsa]$ npm install

npm ERR! code ERESOLVE
npm ERR! ERESOLVE unable to resolve dependency tree
npm ERR! 
npm ERR! While resolving: undefined@undefined
npm ERR! Found: react@18.0.0
npm ERR! node_modules/react
npm ERR!   react@"18.0.0" from the root project
npm ERR! 
npm ERR! Could not resolve dependency:
npm ERR! peer react@"^16.12.0" from pocketbase-react@0.1.3
npm ERR! node_modules/pocketbase-react
npm ERR!   pocketbase-react@"0.1.3" from the root project
npm ERR! 
npm ERR! Fix the upstream dependency conflict, or retry
npm ERR! this command with --force, or --legacy-peer-deps
npm ERR! to accept an incorrect (and potentially broken) dependency resolution.
npm ERR! 
npm ERR! See /home/gani/.npm/eresolve-report.txt for a full report.

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/gani/.npm/_logs/2022-10-13T07_22_56_536Z-debug-0.log

Additionally the above seems to use pocketbase-react, not just pocketbase so to rule out issue with the 3rd party package, please test it first with the plain sdk only.

@ganigeorgiev
Copy link
Member

@tobicrain Here is a minimal js example - https://snack.expo.dev/@jiviba8897/e35de5.

Please test it and if the above works, then probably the issue is somewhere else (most likely a transpiler(s) misconfiguration depending on whether you are using babel, ts, etc.).

@tobicrain
Copy link
Author

isnt working either when running locally, on expo snack its working fine

@ganigeorgiev
Copy link
Member

ganigeorgiev commented Oct 13, 2022

@tobicrain Are you getting the same could not be resolved ... pocketbase.es.mjs error or something completely different? What node version are you using? Could you list all the steps/commands that you are running?

@tobicrain
Copy link
Author

tobicrain commented Oct 13, 2022

npm install; npm run start;

iOS Bundling failed 3526ms
Unable to resolve "pocketbase" from "App.js"

Bildschirmfoto 2022-10-13 um 12 25 33

npm version: 8.5.0
node version: v16.14.2

@ganigeorgiev
Copy link
Member

@tobicrain Thank you. I was able to borrow a newer android device and I've installed expo go and indeed I was faced with the same error during build (strangely there are no errors when I run it with the android studio emulator...).

Anyway, now that I'm able to reproduce the error I'll try to spend some time to debug it and will get back to you soon.

@tobicrain
Copy link
Author

@ganigeorgiev Glad to hear that. Alright, thank you very much

@ganigeorgiev
Copy link
Member

ganigeorgiev commented Oct 13, 2022

@tobicrain I was able to make it work by creating a metro.config.js file and appended the mjs extension to the resolver.sourceExts config:

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

I'm not sure why react native doesn't support the extension by default (it is officially recognized by the node/es specs, including cjs), but in any case the main package.json entry in PocketBase is mainly for supporting the legacy bundlers (older rollup and webpack versions used by nuxt2 for example) and usually should be ignored and the exports property should take precedence but react native seems to ignore that too. If you print the metro config you'll notice also that they seem to support a special react-native package.json property, so I may consider using it in the future and generate a special bundle only for react native without the mjs extension. But for now, the above config should be a good-enough workaround (I've also added a note in the readme).

If you are still not able to make it work, please let me know and I'll investigate it further.

@ganigeorgiev
Copy link
Member

@tobicrain Just for info, I've also pushed a fix in the sdk related to #42, so you may want to update your dependencies.

@tobicrain
Copy link
Author

For Expo React Native now everything works fine 👍🏻
But now when I try to use pocketbase-react in react I'm getting following import error:
Bildschirmfoto 2022-10-13 um 17 08 05

Any idea what the solution could be?

@ganigeorgiev
Copy link
Member

ganigeorgiev commented Oct 13, 2022

How do you load pocketbase-react, using require? Some transform configuration in babel or ts could also be responsible for "converting" es modules into cjs and resulting into the above. I'm not really sure. Could you provide a minimal git repo to reproduce it?

@ganigeorgiev
Copy link
Member

@tobicrain Just to make sure that the error is not caused by my latest changes in v0.7.3 could you try to downgrade your SDK version to v0.7.2?

@ganigeorgiev
Copy link
Member

@tobicrain Just to let you know that in v0.7.4 the additional metro.config.js is no longer required since I've added a react-native prop in the package.json with es bundle without the mjs extension.

@iskaa02
Copy link

iskaa02 commented Jul 17, 2023

@ganigeorgiev
I have a question regrading CommonJS bundle, why only pocketbase Client is bundled why not the other exports?

I'm using React native with mjs and everything works fine, but when testing i have to use the cjs format because jest doesn't support ESmodules, i have been debugging for the past 4 hours trying to get it to work, so i just decided to use the cjs file only for testing, and other errors start popping up, after checking the library source code turned out in rollup.config.js that the common js file only have the client bundled, i had to clone the repo and edit that to work for my use case.

@ganigeorgiev
Copy link
Member

why only pocketbase Client is bundled why not the other exports?

Because as far as I know CJS can't have more than 1 default import, aka. it will require exporting it as some sort of nested object and not just const PocketBase = require(...) and this may break some old bundlers configurations that have "fake" import support which under the hood actually uses the CJS (eg. Nuxt2 and older webpack versions).

In general, the CJS bundle exists just for backward compatibility with older node versions and legacy bundler configurations and the ES bundle should be preferred when possible.

Unfortunately I don't have much experience with Jest and I'm not sure what exactly are its issues with ES modules but there seems to be some experimental support based on https://jestjs.io/docs/ecmascript-modules.

@iskaa02
Copy link

iskaa02 commented Jul 17, 2023

I don't have much knowledge about this, but my issue was fixed editing the rollup.config.js for the cjs fromat

  {
    input: "src/index.ts",
    output: [
      {
        name: "PocketBase",
        file: "dist/pocketbase.cjs.js",
        format: "cjs",
        exports: "named",
        sourcemap: isProduction,
      },
    ],
    plugins: basePlugins(),
    watch: { clearScreen: false },
  }

@ganigeorgiev
Copy link
Member

ganigeorgiev commented Jul 17, 2023

But that's exactly what I meant, aka. you can't have both default and named exports, and the above I think will be returned as an object, which will defer from how the ES bundle is exported.

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

3 participants