-
Notifications
You must be signed in to change notification settings - Fork 353
Add error hint when accessing the default value of a union type with no default #1369
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
base: main
Are you sure you want to change the base?
Conversation
064a391 to
32478ad
Compare
32478ad to
43e9f2e
Compare
| @@ -0,0 +1,17 @@ | |||
| –– Pkl Error –– | |||
| Tried to read property `b` but its value is undefined. | |||
| Union type `Listing<String> | String` (from type alias `B`) has no default member. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This message doesn't explain what "Default member" is. If we want this as the error message, probably should have a suggestion of how to mark a union default member.
But, this error message feels a little strange to me for a couple reasons. This throws too:
foo: Int | Boolean | StringBut having a default member doesn't help, because none of these types have default values.
Also, this error message feels oddly specific; the issue here is that the property doesn't have a default value.
There's no default value because:
- The class property has no declared default value
- The property's declared type has no default value
- The object instance did not assign to this property
So there's three possible resolutions:
- Add an explicit default in the class property (only if this is modifiable code)
- Change the type to have an explicit default (only if one of the member types has a default value, and this is modifiable code)
- Define the property in your instance
I think this current error message would probably be misleading too often.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I updated the condition here to check that the union has no default member and updated the message a bit. How does it look now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It still feels odd to me:
- Adding a default marker doesn't do anything for union types like
Int | Boolean | String - The code might not be modifiable, so we shouldn't have verbage that suggests updating the type as a fix.
To address #1368, it feels like the error message should be something like this?
Cannot instantiate type `Listing<String> | String` because it has no default value.
But we'd really only want to show this error message when we're trying to amend it with an object body.
Resolves #1368