Use undefined instead of null by default

[innocenzi]

So, in TypeScript we use null very sparsely. Usually we use undefined in interfaces.

The difference is confusing because, well, it's JavaScript -- but basically null is when you want to have to explicitly declare a variable with no value, whereas undefined is for cases when a variable should not necessarily be initialized.

Aside from the theory, the practical difference is that you may omit declaring undefined union variables/properties, but you can't omit null union variables/properties, you have to declare them.

For instance, the following would be valid:

interface User {
  id?: number
  name: string
}

const user: User = {
  name: 'John'
}

While the following is not:

interface User {
  id: number | null
  name: string
}

const user: User = {
  name: 'John'
  // Error! `name` is missing
}

For this reason, I think that it would be better to convert nullable PHP types to use undefined instead of an explicit null.

I'm willing to PR but I didn't want to source-dive if you weren't going to agree with that opinion.

[erikgaal]

Hi @innocenzi,

We have this same challenge, but I prefer the way it is solved in https://github.com/spatie/laravel-data/pull/79/files. Instead of replacing all nullable or optional parameters with undefined, I would rather have it explicit by using a special Undefined class.

[innocenzi]

In this PR, the Undefined type is added for the purpose of explicitly creating that type instead of using a null union right? Honestly seems a bit overkill since I'm not sure when you would like to have a null union at all.

But to be honest I'm fine with anything as long as I can generate { property?: Type } interfaces/types.

[rubenvanassche]

Maybe it would be cool if we added an annotation for such types? Like #[Undefinable]? Laravel data version 2 then would have support for this build in by using a union Undefined type.

I would accept such a PR, because at the moment I don't have the time to built it 😬

[spatie-bot]

Dear contributor,

because this issue seems to be inactive for quite some time now, I've automatically closed it. If you feel this issue deserves some attention from my human colleagues feel free to reopen it.

[sebastiandedeyne]

We've always stuck with null because it's more consistent with PHP: when you mark a PHP property as nullable, you still must explicitly provide the value.

class User extends Data
{
    public function __construct(
        public int $id,
        public ?string $name
    ) {}
}

new User(id: 1); // Error!

type User = {
  id: number;
  name: string | null;
}

const user: User = { id: 1 } // Error! Consistent with PHP.

If we'd transform this to undefined, you could instantiate a user in TypeScript without providing a user. If we'd change the behavior to transform nullable types to undefined we lose the current ability: there would be no way to explitly require a nullable property to be set.

What could be treated as undefined, are nullable properties with a default value.

class User extends Data
{
    public function __construct(
        public int $id,
        public ?string $name = null
    ) {}
}

new User(id: 1); // Fine!

type User = {
  id: number;
  name?: string | null;
}

const user: User = { id: 1 } // Fine! Consistent with PHP.

Not sure if this is a path worth exploring, of if it's too implicit.

Sidenote: in practice we often solve this by using TypeScript's Partial generic for forms on the frontend.

Personally I'm not convinced making this behavior configurable is worth it. But if there's enough demand and it doesn't introduce too much complexity I'm not against it either.

[innocenzi]

@sebastiandedeyne I think you're too focused on PHP semantics. What's important is that TypeScript types are used on the front-end.

This makes me think this is because of how Vue's types for the DOM are written (and, in my opinion, this is the correct way), and that you're not encountering that problem with React (I know you use mainly React at Spatie).

Specifically, take a look at runtime-dom.d.ts. This is what Volar uses to typecheck properties of HTML elements, and diverse DOM APIs.

Code examples

This simple one shows that string | null is not assignable to the attribute title, because it expects string | undefined:

<script setup lang="ts">
// generated by Laravel Data
interface User {
	full_name: string | null
}

const user: User = { full_name: 'Jon Doe' }
</script>

<template>
	<div>
		<input :title="user.full_name" type="text" />
	</div>
</template>

This other one shows a custom component with a v-model directive that expects a string | undefined value (the default when you only specify string and it's "nullable"):

This is the component:

<script setup lang="ts">
const text = defineModel<string>()
</script>

<template>
	<div>
		<input type="text" v-model="text">
	</div>
</template>

This is how the component is used:

<script setup lang="ts">
import TextInput from './text-input.vue'

// generated by Laravel Data
interface User {
	full_name: string | null
}

const form = reactive<User>({
	full_name: 'Jon Doe'
})
</script>

<template>
	<div>
		<TextInput v-model="form.full_name" />
	</div>
</template>

This one is using a random VueUse API:

// generated by Laravel Data
interface User {
	created_at: string
	date_format: string | null
}

const $props = defineProps<{
	user: User
}>()

const createdAt = useDateFormat($props.user.created_at, $props.user.date_format)
// the second param here fails with a null union but works with an optional property

Here's a kinda dumb example, but to showcase how some Web APIs don't support null but support undefined:

// generated by Laravel Data
interface Settings {
	timeout: number | null
}

const $props = defineProps<{
	settings: Settings
}>()

setTimeout(() => {
	// do smth
}, $props.settings.timeout) // this errors with null unions but not optional properties

While some of the above examples, or other situations are easily worked around, this is a recurring behavior that would be easily fixed if we could make all nullable properties optional instead. Some of these examples are also not-so-easily worked around, like the custom component using v-model.

Generally speaking, either with VueUse, random JavaScript libraries, or even the DOM API itself: most of them use optional properties in their interfaces. There are a few examples where null is also supported, but these are the exceptions, not the rule.

So, yeah—while I understand the consistency argument, I don't think it's actually relevant (or if it is and I missed actual use cases, please tell me). So an option to support this behavior would be more than welcome!

[sebastiandedeyne]

Okay, I'm convinced 😉

Another reason I've changed my mind is this:

test('data validation', function () {
    class UserData extends Data
    {
        public function __construct(
            public string  $email,
            public ?string $name,
        ) {}
    }

    Route::post('users', function (UserData $data) {
        return $data;
    });

    post('users', ['email' => '[email protected]'])
        ->assertSessionDoesntHaveErrors();
});

I actually expected this to fail. I thought not having a default value would cause data validation to fail. (I thought laravel-data's behavior was to require the presence of a value for nullable types.) Since this isn't the case, having types marked as undefined wouldn't be as inconsistent with data as I thought. Although it would still be inconsistent with raw PHP semantics, but some I agree some pragmatism is allowed.

I'm still wary of having this as the sole behavior though, to be safe I'd (at least for the next major) have it configurable.

[innocenzi]

Really glad to read that! Having it globally configurable would be a great compromise.