tinyplex / tinybase

The reactive data store for local‑first apps.
https://tinybase.org
MIT License
3.23k stars 65 forks source link

Hermes error with Expo 50 + Tinybase v5 beta #148

Closed mdj-uk closed 3 weeks ago

mdj-uk commented 1 month ago

Describe the bug

Trying to import from "tinybase/with-schemas" or "tinybase/lib/with-schemas" in an expo app I get the following error:

Metro error: Unable to resolve module tinybase/lib/with-schemas from myprojectpath/app/(tabs)/index.tsx: tinybase/lib/with-schemas could not be found within the project or in these directories:
  node_modules
  ../node_modules
   5 | import { ThemedText } from "@/components/ThemedText";
   6 | import { ThemedView } from "@/components/ThemedView";
>  7 | import { createStore } from "tinybase/lib/with-schemas";
     |                              ^
   8 |
   9 | const store = createStore()
  10 |   .setValuesSchema({

Your Example Website or App

https://github.com/mdj-uk/cea

Steps to Reproduce the Bug or Issue

Clone and run (npm run start) the github repo above. Or:

  1. npx create-expo-app myapp

  2. cd myapp; npm run start

  3. npm install tinybase --force or npm install tinybase@4.8.0 *

  4. Add the following to app/(tabs)/index.tsx:

    import { createStore } from "tinybase/lib/with-schemas";
    
    const store = createStore()
     .setValuesSchema({
       employees: { type: "number" },
       open: { type: "boolean", default: true },
     })
     .setTablesSchema({
       pets: { species: { type: "string" } },
       species: { price: { type: "number" } },
     });
    
    store.setValues({ employees: 12, open: false });
    store.setTables({
     pets: { fido: { species: "dog" } },
     species: { dog: { price: "5" } },
    });
    
    const e = store.getValue("employees");
    console.log(e);
  5. open app in android device or browser

* there's a peer dependency issue with the latest version, since it expects react 18.3 but the expo starter app uses 18.2. So either force install, or use 4.8.0 instead which has no peer dependency issue

Expected behavior

No response

Screenshots or Videos

No response

Platform

Additional context

Note that there's no runtime/build error with import { createStore } from "tinybase" - but then you don't get the full typescript support. (e.g. it doesn't warn that price of dog should be a number not a string)

I've tried the following imports but none seem to work:

jamesgpearce commented 1 month ago

Ugh, why are the RN imports so fiddly? I can look at this tomorrow.

jamesgpearce commented 1 month ago

I think the basic problem is that the library is in lib but the types are in lib/types/with-schemas. Most bundlers get to know about this split from package.json but apparently not Metro. I'll see if I can find a way to get it to work.

mdj-uk commented 1 month ago

Ugh, why are the RN imports so fiddly? I can look at this tomorrow.

My sympathies! Thanks for taking a look

jamesgpearce commented 1 month ago

OK, I have a horrible thought that the only way to fix this is to relayout the structure of the distributed package so that it matches the import pattern literally (rather than relying on the exports field in package.json).

It should be a barely breaking change but I think I need to put it in the forthcoming v5 release. Would you be OK with trying out a beta of that once I make these changes?

mdj-uk commented 1 month ago

Update: I've realised that enabling package exports support by setting config.resolver.unstable_enablePackageExports = true; in the metro config file solves the issue.

So I wonder if just documenting that and asking users to enable the setting would be a better solution than having to make a drastic change to the package? (I'm afraid I don't have enough experience to know what problems that setting might cause. In a simple starter app everything is fine.)

But if you make the change I'll be very happy to try the beta

Thanks again

jamesgpearce commented 1 month ago

That was going to be my other suggestion (but when I tried it it didn't work for me!). Glad you're unblocked though!

jamesgpearce commented 4 weeks ago

I'm hoping we are past these sorts of issues in v5.0.0-beta.21

It seems to work great in your app (though still currently needs the --force install). If you get a chance could you give it a whirl yourself please? Thanks!

mdj-uk commented 3 weeks ago

Hi, as far as I can tell the latest beta works perfectly with expo sdk 51.

I'm afraid it seems to break apps using tinybase with expo 50 though.

As soon as I add the following

import { createStore } from "tinybase";

const store = createStore();

I get an error when running on a (real) android device. Web seems ok. (I'm not able to test iOS)

The not-very-helpful error message:

 ERROR  TypeError: Cannot read property 'prototype' of undefined, js engine: hermes 
    at ContextNavigator (http://192.168.1.17:8081/node_modules/expo-router/entry.bundle//&platform=android&dev=true&hot=false&lazy=true&transform.engine=hermes&transform.bytecode=true&transform.routerRoot=app:149934:24)
    at ExpoRoot (http://192.168.1.17:8081/node_modules/expo-router/entry.bundle//&platform=android&dev=true&hot=false&lazy=true&transform.engine=hermes&transform.bytecode=true&transform.routerRoot=app:149890:28)
    at App
    at ErrorToastContainer (http://192.168.1.17:8081/node_modules/expo-router/entry.bundle//&platform=android&dev=true&hot=false&lazy=true&transform.engine=hermes&transform.bytecode=true&transform.routerRoot=app:194841:24)
    at ErrorOverlay
    at withDevTools(ErrorOverlay) (http://192.168.1.17:8081/node_modules/expo-router/entry.bundle//&platform=android&dev=true&hot=false&lazy=true&transform.engine=hermes&transform.bytecode=true&transform.routerRoot=app:194591:27)
    at RCTView
    at View (http://192.168.1.17:8081/node_modules/expo-router/entry.bundle//&platform=android&dev=true&hot=false&lazy=true&transform.engine=hermes&transform.bytecode=true&transform.routerRoot=app:40188:43)
    at RCTView
    at View (http://192.168.1.17:8081/node_modules/expo-router/entry.bundle//&platform=android&dev=true&hot=false&lazy=true&transform.engine=hermes&transform.bytecode=true&transform.routerRoot=app:40188:43)
    at AppContainer (http://192.168.1.17:8081/node_modules/expo-router/entry.bundle//&platform=android&dev=true&hot=false&lazy=true&transform.engine=hermes&transform.bytecode=true&transform.routerRoot=app:39999:36)
    at main(RootComponent) (http://192.168.1.17:8081/node_modules/expo-router/entry.bundle//&platform=android&dev=true&hot=false&lazy=true&transform.engine=hermes&transform.bytecode=true&transform.routerRoot=app:124871:28)

I've added a branch to my reproduction repo which downgrades from expo 51 to 50 and uses the latest tinybase beta. If you uncomment the lines above in app/(tabs)/index.tsx you'll see the error. Link: https://github.com/mdj-uk/cea/tree/expo-50 (Downgrading to tinybase@latest there's no problem.)

jamesgpearce commented 3 weeks ago

what the what

jamesgpearce commented 3 weeks ago

Yeah I can repro this on iOS too. Error indicates it doesn't like this integer 🤔 but more likely it's the TextEncoder above it...

image
jamesgpearce commented 3 weeks ago

OK, so I've confirmed that TextEncoder was only added in SDK 51. Blog post here.

I am going to add an expo SDK peer dependency to stop people running into this too hard, but if anyone has to run SDK50 or below and forces past the peer requirements, it's possible to work around this issue with the text-encoding polyfill:

npm install text-encoding

And then in the file that imports TinyBase v5:

import 'text-encoding';

cc @brentvatne