Node of kind optional is not valid as a required definition

Hello! Whenever I use a type with an optional key with a default value like this:

const def = type({
    'agree?': 'boolean = true',
})

const def = type({
    'agree?': 'boolean = true',
})

I get the error Node of kind optional is not valid as a required definition. What could it mean? Note that I'm also updating the type via map, but removing that mapping appeared to have no effect so far. Removing the default value throws no error

22 Replies

ssalbdivad•3mo ago

Hmm that isn't a great error and I'd like to just ignore the redundant optionality here. It used to be an error to do this as the default value already implies optionality, but I loosened those restrictions a lot. Maybe you could create an issue for the confusing error here? But the standard way to write this would just be:

const def = type({
    agree: 'boolean = true',
})

const def = type({
    agree: 'boolean = true',
})

PIatOP•3mo ago

I got to test it out now. Doing this:

const testDef = type({ 'agree?': 'boolean = true' })
console.log(testDef({ agree: false }))

const testDef = type({ 'agree?': 'boolean = true' })
console.log(testDef({ agree: false }))

doesn't throw any error It seems like the Node of kind optional... error is related to the way I mapped the type somehow

ssalbdivad•3mo ago

Oh yeah that makes sense What does that look like?

PIatOP•3mo ago

I'll try to create a minimal example, hopefully I'll find the exact culprit that way 🙏

ssalbdivad•3mo ago

My guess would be maybe it involves spreading a node or something. I think I added some special case for handling that, but it checks out there could be either something I'm not handling internally about that mapping or an external mistake related to mapping optionality

PIatOP•3mo ago

Yes, you're right! This is distribute is the culprit:

const testDef = type({ 'agree?': 'boolean = true' })

testDef.in.internal.distribute(
    (branch) => {
        return branch.required().get('intent').unit
    },
    (branches) =>
        branches.reduce((acc, intent) => ({ ...acc, [intent]: intent }), {}),
)

const testDef = type({ 'agree?': 'boolean = true' })

testDef.in.internal.distribute(
    (branch) => {
        return branch.required().get('intent').unit
    },
    (branches) =>
        branches.reduce((acc, intent) => ({ ...acc, [intent]: intent }), {}),
)

I have my grievances with it as it is, since it also makes it so that if some branch doesn't have intent, it throws

ssalbdivad•3mo ago

Yeah this looks a bit cooked haha I mean Hmm .get("intent").unit is the biggest thing that seems not safe Because it could be a node

PIatOP•3mo ago

It would also throw with Node of kind optional... in

const testDef = type({ 
  intent: "'update'",
  'agree?': 'boolean = true'
})

const testDef = type({ 
  intent: "'update'",
  'agree?': 'boolean = true'
})

It doesn't manage to trip on intent not actually being there

ssalbdivad•3mo ago

If there is a specific repro for something that seems broken maybe log an issue. I wonder if maybe some of the issues are related to the proto node issues I mentioned with Date

PIatOP•3mo ago

I seems in this case that having .required at all is the problem Removing .required makes the code work:

const testDef = type({
    intent: "'update'",
    'agree?': 'boolean = true',
}).or({
    intent: "'delete'",
})

const intents = testDef.in.internal.distribute(
    (branch) => {
        return branch.get('intent').unit // get intent without `required`
    },
    (branches) =>
        branches.reduce((acc, intent) => ({ ...acc, [intent]: intent }), {}),
)

const testDef = type({
    intent: "'update'",
    'agree?': 'boolean = true',
}).or({
    intent: "'delete'",
})

const intents = testDef.in.internal.distribute(
    (branch) => {
        return branch.get('intent').unit // get intent without `required`
    },
    (branches) =>
        branches.reduce((acc, intent) => ({ ...acc, [intent]: intent }), {}),
)

Should I file the issue with this example?

ssalbdivad•3mo ago

Yeah I suppose. None of this really makes a ton of sense to me lol but still if something is getting parsed in a way it shouldn't that is good to know. The .assertHasKind("unit") function might be helpful in making things more robust

PIatOP•3mo ago

Will look into it 👍

ssalbdivad•3mo ago

Basically just get('intent').assertHasKind("unit").unit So you'd get a clear message right away if that wasn't the case

PIatOP•3mo ago

How could it be made to not throw if intent isn't present? To just return null

ssalbdivad•3mo ago

You could check if branch.keyof().allows("intent") first I guess

PIatOP•3mo ago

Thank you, yeah I would've never guessed allows 😁

ssalbdivad•3mo ago

allows is the validation method that gets called the most- any time you invoke a type Because it's much more efficient if the data is valid Then .traverse which keeps track of path and context etc. is only called if allows returns false to give a good error message and apply morphs etc. I guess you don't really have to worry about any of that externally though haha

PIatOP•3mo ago

I see! Thank you! Well, in my edge-casy use-cases it's good to know these things 😅

ssalbdivad•3mo ago

Yeah, any time you can check a literal value like "intent", you skip a lot of overhead treating as a value instead of a type It's the same as checking:

const intent = type("'intent'")

intent.extends(branch.keyof())

const intent = type("'intent'")

intent.extends(branch.keyof())

But you skip having to create the type for it and also some unnecessary internal hoops for actually managing the type comparison

PIatOP•3mo ago

The context deepens Thank you ☺️ I also filed the issue, hope it helps in any way

ssalbdivad•3mo ago

This is fixed in 2.0.0-rc.14

PIatOP•3mo ago

Thank you!

Gaming

Programming

Node of kind optional is not valid as a required definition