Convex ents v.optional() makes user Value | undefined
Hi guys, when I add
.field('subscriptionId', v.optional(v.string())) to my schema all values suddenly become Value | undefined, but when there are no optional fields no errors arise. How could this be fixed?
60 Replies
Here is my schema.ts without it:
If a field is optional, it's type will be a union with
undefined because the value may actually be undefined. The errors are a feature, the compiler is telling you have to make sure the value is defined in those places.
For example, stripeApi.customers.retrieve() requires a string, and will probably throw an error if you pass it undefined. There are many ways to conditionally check for undefined to fix these errors, depends on your use case and the situations in which this field would be undefined.
So, eg., if you have a form that should only ever be viewable by a user that has a stripe id, you can query the user and then throw right away if they don't have a stripe id. The throw should never occur since the user shouldn't be able to get to the form without a stripe id anyway, and Typescript can then deduce that, after the conditional throw, the user definitely has a defined stripe id.Hmm, but I am talking about all of the values of the user become
Value | undefined shouldn't only the subscriptionId itself be Value | undefined??In the error from your screenshot,
user.stripeUserId is Value | undefined
Do you have an error where user itself is union w/ undefined?Yes thats what I am talking about when I make the subscription id optional
.field('subscriptionId', v.optional(v.string())) like so everything else becomes Value | undefined even the stripeUserId which is not optional.When I remove the optional from the subscriptionId everything works fine:
When I do not make the subscriptionid optional then no, but when I do:
Uhh, sorry If I did not provide enough info. Is this supposed to happen? Shouldn't only the optional field be
| undefined???
I am talking about if I make the subscriptionId field optional all other fields like stripeId which are not optional become Value | undefined .aahh I see what you're saying now
Can you share your schema with optional field set
Is it just
{ optional: true } on that one field and no other changes?
I haven't used Ents much, and would need to look more into the docs to confirm, but you may need to use optional: true instead of v.optional()
hmm actually that looks like it's edge specific
yeah that should work
with optional
May have to wait for @Michal Srb to evaluate, not certain if this is a bug but it seems like it.
well If do it like in that screenshot, that makes everything else
Value | undefinedFields are not made to be used like that because they are all indexed, i don't see why you would want to make all those fields indexed, i only use them for something i want to be unique.
Not sure what you mean here, they're just defining a schema
In every example ive seen there is quite a bit of mixing and matching. so i understand field to not be a full replacement, otherwise it would be used as such. from the saas-starter template. users: defineEnt({
firstName: v.optional(v.string()),
lastName: v.optional(v.string()),
fullName: v.string(),
pictureUrl: v.optional(v.string()),
})
.field("email", v.string(), { unique: true })
.field("tokenIdentifier", v.string(), { unique: true })
.edges("members", { ref: true, deletion: "soft" })
.deletion("soft"),
Ah I see, didn't realize you could define fields for ents without
.field(), or that .field() creates an index. That said, I still wouldn't expect the behavior OP is seeing.Looks like a bug
Even after changing my schema to following:
the issue persists
Oh, I think you have a bug in your query?
Can you share the full code of getUserByExetrnalIdOrThrow, and make sure it has no type errors?
I'll have a look later today, does look like a bug in Ents
Okay, thank you very much!
Also is it recommended to mix and match like this or use
.field() for everything? Like if I do not make my field unique its not indexable.It shouldn't matter which syntax is used. Fields from either syntax can be used in indexes.
Did you figure this out? Are the issues on my side?
I think this must be caused by some usage of the field that you're making optional. Can you look at every place where you're using the field, and see if making it optional doesn't break anything? Are you using it in your custom query or mutation definition?
Even the original error made sense, the function requires string, but you were passing in a
string | undefined, so that should error (although I'm not sure why the error message says Value | undefined).But in the original error I was passing the
stripeUserId which is not optional, only the subscriptionId is.
I was using the subscriptionId nowhere yet, by just simply adding it it made all other fields undefined.Here is a demo video:
Can you try two things:
1. In
getUserByExternalIdOrThrow change the last line to return user.doc(); (what happens now, do you still have the same issue?)
2. at the getUserByExternalIdOrThrow callsite, change const user = to const user: Doc<"users"> = (what happens now, do you still have the same issue?)
Even if either resolves the issue, if you could share a repo with the reproduction, that would be super hepful as I would love to know why adding the field changes things.using
Doc<"users"> (I have it imported as Document_, just to avoid confusion).
And with using
.doc() the same issue persists as before.For the second point, you need to annotate where you have your
runQuery in the action, not inside of the query.Also tried still the same issue.
Could you put together a repo on GitHub that I can clone and replicate the error?
Sure, also just a quick question, I just recently updated to
convex-ents 0.7.3 from convex-ents 0.7.2. And I had to remake my upsert function to look like this:
from this:
Is this expected in the update? I need to use
.getX() to be able to use .patch(), .delete() etc.. on the user in the new update.
I will make the repro but I just want to clarify if this is intended in the update?
I am unsure which version to use in the repro, please tell me if this is an expected behavior.That's probably broken in the update, but shouldn't affect your issue. Feel free to use the latest and I will look at both issues.
Uh, if that is not expected then I will probaly share the repo which uses 0.7.2. I just have to make a few modifications to it. Sorry for taking so long.
No worries, no rush
Is it okay if I share slightly modified version of my full repo only with you?
Okay, so Imade a heavly stripped down version of the app which has nearly the same functionality and is in english. Here is the repro repo: https://github.com/kanepop/private-repro-convex
I am pretty new to convex so if you want if find something to be improved or use better practices feel free to tell me.
I sent you an invite did you check it out?
I got the invite, looking
Thank you very much.
I cloned the repo and ran
npm i. Then I ran npx tsc and I got no error. I also have no error in VS Code.
Which TypeScript version are you using? You can click on the {} button as the screenshot shows.
You must have 5.4.x at least. (If you update VS Code it will come with 5.4.x)
Well, oh, it is a dev version. VS Code had this set for me as default.
Even after switching to 5.4.5 it persists.
Also sorry but I forgot to add this to the schema:
.field('subscriptionId', v.optional(v.string())),.
If you add this field it starts happening
I will make a commit with the field added to the schema.Right, I forgot that, got it reproed
I made the commit.
Also feel free to ping me when you find the issue. And again thank you very much.
Ok, so part of the problem is
exactOptionalPropertyTypes in your convex/tsconfig.json.
I'm super curious where you got your tsconfig and eslint setup from. It's trying to be super super strict?
Yup, I think removing that setting fixes things and you can use convex-ents@0.7.4I used these configs with a friend when we were working on project a while ago.
Oh, a such stupid mistake haha, I should have not blindly used those config. I am sorry for using up your time on stupid issues. And again thank you.
We spent some time optimizing these templates: https://github.com/get-convex/templates/tree/mike/redesign/template-nextjs-clerk-shadcn
So they have pretty good eslintrc and tsconfig setups (not overly strict - but you can make it stricter by deleting the overrides in the eslintrc as hinted).
GitHub
templates/template-nextjs-clerk-shadcn at mike/redesign · get-conve...
Contribute to get-convex/templates development by creating an account on GitHub.
Will take a look, thanks.
do you know if anyone is using the new "flat" setup with eslint.config.js / mjs ?
We are just having a nightmare trying to get eslint to work on the convex folder. i have double checked, triple checked. debugged, vscode shows this error. Error: Could not find config file. in the eslint output but things like eslint -debug are fine. probably like this: https://github.com/eslint/eslint/issues/17784 . i feel like mono-repos are just designed for punishment.
GitHub
Bug: Error: Could not find config file. · Issue #17784 · eslint/esl...
Environment Node version: v16.20.0 npm version: v9.6.6 Local ESLint version: Not found Global ESLint version: Not found Operating System: darwin 23.0.0 What parser are you using? Other What did you...
@ampp I’m not familiar with “flat” setup. Monorepos are indeed tricky atm.
I think he means: https://eslint.org/blog/2022/08/new-config-system-part-2/
ESLint's new config system, Part 2: Introduction to flat config - E...
A pluggable and configurable linter tool for identifying and reporting on patterns in JavaScript. Maintain your code quality with ease.