Math.random unsupported while evaluating schema
I'm working on my
Effect wrapper library for Convex, and discovered through this work that importing a module which uses Math.random produces the following error when running convex dev:
1. Is there any way for me to work around this right now?
2. Would it be possible to treat Math.random in this context in the same way that it is treated in Convex functions?41 Replies
What do you need Math.random for when creating a schema if you dont mind me asking?
I don't, but it's being invoked by a dependency (although I am certain that its results are not used in the context in which I'm using said dependency)
Ah, its one of those top level module import usages of Math.random? fun!
Im sorry I dont have any answer for to help you with this one RJ other than dont use that Dependency.
Hopefully a Convex team member will be able to offer more suggestions
We chose not to implement this in the schema environment because it seemed like you shouldn't use it there — and we wanted it to fail in an obvious way instead of always returning 0 because silently failing could be a security issue.
Re (1) is there any way to work around it: I'm not sure, any approach like patching Math.random to have different behavior seems like a bad idea because you might do that patch when schema.js is imported by other files, causing Math.random() not to work.
Re (2) would it be possible to treat Math.random the way it is in other contexts: yes it's possible, and the implications don't sound that much worse that using Math.random() for a global variable that other behavior is based on when e.g. defining mutations or queries.
@RJ would love to hear how it's coming up, treating it other ways isn't out of the question; if it's blocking work on your library that's important
You evidently didn't anticipate such profane use cases as mine 🦹♂️
This would work beautifully for my needs here, I think
Here's where it's coming up:
- As the seed to a random number generator class: https://github.com/Effect-TS/effect/blob/e0ef64102b05c874e844f680f53746821609e1b6/src/Utils.ts#L541
- Which is instantiated for use in the
Hash module: https://github.com/Effect-TS/effect/blob/e0ef64102b05c874e844f680f53746821609e1b6/src/Hash.ts#L16
- Which is used for efficient comparison of data (like Rust's Hash trait, I believe, or Haskell's Hashable)
So treating Math.random() as its treated in Convex functions should therefore work fine here, in the sense that it shouldn't cause any issues with how the Hash trait (which seems to be what the Effect folks are calling this sort of thing) functions
It does also appear here: https://github.com/Effect-TS/effect/blob/e0ef64102b05c874e844f680f53746821609e1b6/src/internal/defaultServices.ts#L25 to see the Random service (so basically, to seed the random number generator which one is expected to use in the context of an Effect computation, in order to have easy determinism for e.g. testing). But in this context (defining the schema) I don't see any reason why the Random service should be used; whereas in theory the Hash trait could be utilized sensibly here.
And to be clear—this issue comes up anytime any Effect dependency is imported, so it makes the library unusable in the Convex schema.ts contextCould both the library and schema.ts import table definitions from a shared module with no dependencies as a workaround?
Don’t think that I have anything meaningful to contribute besides sharing that I’ve also experienced this with effect. Sorry if this is just noise, but the hope is to produce some data wrt how common/uncommon of an issue this is.
Helpful to hear @em_errata, was this also for Schema?
It was, yes.
sorry and to clarify, also for schema.ts?
No, that’s a good clarification! Yes, I was attempting to co-locate my validation logic within the schema module tree.
Possibly I’m not understanding your suggestion, but I think I’d have to do some codegen to make that sort of thing work.
The flow is basically:
- Create Effect
Schema version of your Convex schema
- Generate the Convex SchemaDefinition from the Effect Schema version
Which means that there’s inevitably a dependency chain from Schema to schema.ts. I guess I could come up with some DSL for defining both schemas which depends on neither… but then that defeats a main purpose of this library!Gotcha. The minimum code
schema.ts needs to import pulls in the Hash module. I thought there was a layer where Hash-related code could import from a schema def which could be shared with schema.ts. I'll leave it to Tom hereAlas no, but I appreciate the suggestion!
For anyone interested, here's the full dependency chain:
- User imports the
Schema module
- Which imports the Equal module: https://github.com/Effect-TS/schema/blob/1e115afab16c79841f683f73b51357805d8bf39e/src/Schema.ts#L11
- Which imports the Hash module: https://github.com/Effect-TS/effect/blob/e0ef64102b05c874e844f680f53746821609e1b6/src/Equal.ts#L5
- Which calls new PCGRandom() at the top-level: https://github.com/Effect-TS/effect/blob/e0ef64102b05c874e844f680f53746821609e1b6/src/Hash.ts#L16
- And PCGRandom (imported from Utils) uses Math.random() in its constructor: https://github.com/Effect-TS/effect/blob/e0ef64102b05c874e844f680f53746821609e1b6/src/Utils.ts#L541
Curious to know if there's a rough ETA for a fix here. I've been sorta holding off on working on theeffect-convex library further until this gets resolved. Is this the sort of thing I could expect to be supported on a timescale of a couple weeks, or a couple months (or other)?Let's say weeks, I posted a PR for it last week but @lee had some good suggestions for it I haven't implemented yet
regarding how we treat the random seed — nothing big, I've just been having a good time focusing on a another project
darn you @lee and your commitment to high-quality engineering!
but seriously sounds great, thank you!
As of the latest Convex release, I'm no longer blocked by
Math.random limitations, but am now encountering into this issue: https://discord.com/channels/1019350475847499849/1157442906332856381
I'm of course importing Schema in my convex/schema.ts file, like so:
And also an additional 250 lines of library code (../../src/schema). The whole convex/schema.ts file looks like this:
Sorry about this RJ and thanks for reporting as you explore this direction. What you're doing is reasonable and we'll look at supporting it tomorrow.
So far we've encouraged folks to make the schema bundle as small as possible so hadn't hit this much but this is not a requirement, we can change it.
@RJ This is resolved, the bundle produced from schema.ts has the same limits as the regular code bundle.
Thanks @ballingt! I'll report back when I have some time to try it out
The latest error; pretty self-explanatory I think:
Would you be willing to give uses of
Date the same treatment as for Random in queries and mutations?Hmm maybe we can add importing
@effect/schema in a schema to our test suite to ensure this works! Yes this can get the same treatment.Could pair with a warning like:
It looks like code in your schema file, or one of its dependencies, uses methods from theYeah that would be great, that's a much shorter feedback loop 🙂 Did this ever happen @ballingt? I'm on the latest version and am still seeing this error come up!Dateobject. This may not be a problem, but do note that when you deploy your Convex schema,Date.now()will be set as the time your schema was deployed, and will not change thereafter.
That's looks right RJ, checking in now about when we could slip this in.
Sorry to pester you @ballingt, but if you could let me know once you have some idea when this might be shipped, I would really appreciate it! I have a bit more free time now than I have in the last couple of months, and am eager to work on this more 🙂
Absolutely will!
Friendly bump 🙂
RJ, I'm thinking about this but not actively working on it
No problem @ballingt, appreciate you checking in 🙇♂️ I got this far enough along last month that I think I could ship a v0.1, if I could test it/import
Schema in convex/schema.ts, so I'm just excited to give that a shot! But no stress, I'm sure y'all have lots of other more important things to get done
Turns out that for testing, at least, I can just do
😶Just wanted to give a big upvote to this one here @jamwt.
While waiting for components, Effect-TS has significantly simplified my async workflows in Convex
I don't understand the complexity of what needs to be done here and know you guys have a lot on your plate though 🙏
It works fine as is, but I'd love to see @RJ continue his work so I can swap out zod for Effect schemas 🎉
Date is now supported at Schema evaluation time. We haven't tried Effect yet, but hopefully it removes one more impediment. This was purely serverside, so you won't need to update the npm package. From our internal PR notes: "set Date.now to a fixed timestamp determined at the beginning of schema evaluation"
Fantastic, thanks @Indy et al.! I'll try it out soon and report back
Resolved!
Excellent work!
@RJ did you end up writing some helpers for using Effect (and effect schema) with Convex?
This looks pretty great!
I’m excited to dig in now that I’m back in Convex
@RJ why do we have to explicitly define return types? Couldn't we infer those?
Sort of; there are mainly two reasons why it's always required to add a
Schema for returns validation:
1. It's more secure, because a) you can be sure that, by modifying a Confect/Convex function, you don't accidentally begin exposing additional data that you shouldn't be, and b) since TypeScript doesn't support exact types, it's possible to return data that's not reflected in the inferred return type
2. If you want to return e.g. an Effect type like Option, you need to serialize it as it crosses the wire, so we need to know how to do that. If you just try to return values like an Option without doing so, things might not work how you expect them to (or at all)
If you haven't noticed already, you can pull the Schemas for your DB schema like so: https://github.com/rjdellecese/confect/blob/a5913f04524a0431b25edd9701ff9cb70f8d7497/test/convex/functions.ts#L19
In case that makes things easier2. Is what would require it I imagine.
I wasn’t planning on returning anything but serializable types though - which would also allow me to work with the standard convex clients (and the ones for start).
I guess if you are aware of the risks and stay away from trying to return types like
Option then it's technically not required
When you say "standard Convex clients" do you mean you just want to write your frontend exactly the same as you would without Confect (e.g. don't worry about decoding return values, maybe don't even use Effect in the client)?I'm setting up my current project with TanstackStart and using the Tanstack Query integration.
I'll use Effect on the client, but I'm okay expecting serialized values from the server and then just picking it up fromt here
I like the idea of doing effect through and through, but the maintenance overhead of that seems pretty steep. Especially with the upcoming "local first" and "tanstack" apis
You're referring to the need to separate the
args/returns schemas and reference them when invoking the function, like here, yeah? https://github.com/rjdellecese/confect/blob/a5913f04524a0431b25edd9701ff9cb70f8d7497/example/src/App.tsx#L93-L97
Just want to make sure I'm not missing some other pain point(s)
Assuming so I definitely agree, it's pretty tedious
But
You don't need to use these @rjdellecese/confect/react helpers if you don't want to
You can just use the exposed Convex functions like normal, with TanStack Query or whatever else uses the vanilla APIs, if you don't need/want to do any deserializing in the client
Or you can do it later
I do also plan on making this better in the future, it's just a bit more involved because I'll need to do some codegen
And I'd like it to be integrated with TanStack Start, at a minimum, because that's what I'll be moving my project over to 🙂