Skip to content
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

Dot-notated paths passed to the useProperty become invalid when reaching for a key past a nullable key #57

Closed
5 tasks done
nhedger opened this issue Mar 5, 2023 · 1 comment · Fixed by #58
Closed
5 tasks done
Labels
priority: low Nice to have, enhancement

Comments

@nhedger
Copy link
Contributor

nhedger commented Mar 5, 2023

Describe the bug

Dot-notated paths passed to the useProperty composable are valid only until a nullable key is reached.

Reproduction

https://github.com/nhedger/hybridly-useProperty-issue

Steps to reproduce

Create the following data objects.

<?php # app/Data/GlobalProperties.php

namespace App\Data;

use Spatie\LaravelData\Data;

class GlobalProperties extends Data
{
    public function __construct(public readonly SecurityData $security) {}
}
<?php # app/Data/SecurityData.php

namespace App\Data;

use Spatie\LaravelData\Data;

class SecurityData extends Data
{
    public function __construct(
        public readonly ?UserData $user,
        public readonly string $test,
    ) {}
}
<?php # app/Data/UserData.php

namespace App\Data;

use Spatie\LaravelData\Data;

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

Try accessing any key on the user

Try accessing any key on the user and see that it produces TypeScript error TS2345.

// TS2345: Argument of type '"security.user.id"' is not assignable to parameter of type 'Path '.
const name = useProperty('security.user.id');

// TS2345: Argument of type '"security.user.name"' is not assignable to parameter of type 'Path '.
const name = useProperty('security.user.name');

// TS2345: Argument of type '"security.user.email"' is not assignable to parameter of type 'Path '.
const name = useProperty('security.user.email');

System information

System:
    OS: macOS 13.1
    CPU: (16) x64 Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
    Memory: 4.99 GB / 32.00 GB
    Shell: 5.8.1 - /bin/zsh
  Binaries:
    Node: 19.3.0 - ~/Library/Caches/fnm_multishells/40931_1677913160299/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 9.2.0 - ~/Library/Caches/fnm_multishells/40931_1677913160299/bin/npm
  Browsers:
    Firefox: 110.0.1
    Safari: 16.2
  npmPackages:
    hybridly: link:../hybridly/packages/hybridly => 0.1.0-alpha.2

Used package manager

npm

Logs

No response

Validations

  • Read the docs.
  • Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
  • Make sure this is a Hybridly issue and not an issue related to something else (Vite, Vue...). For example, if it's a Vue SFC related bug, it should likely be reported to vuejs/core instead.
  • Check that this is a concrete bug. For Q&A open a GitHub Discussion.
  • The provided reproduction is a minimal reproducible example of the bug.
@nhedger nhedger added the pending triage This issue needs a proper label to be attributed label Mar 5, 2023
@nhedger nhedger changed the title Dot-notated paths passed to the useProperty become invalid when reaching a nullable key. Dot-notated paths passed to the useProperty become invalid when reaching for a key past a nullable key Mar 5, 2023
@nhedger
Copy link
Contributor Author

nhedger commented Mar 5, 2023

I believe that the following two problems need to be solved:

  1. The type of the path should be fixed in a way that it traverses the whole target type, regardless of nullable keys.

  2. The return type of useProperty should be either the type of the final fragment (e.g. SecurityData when using useProperty('security')) or a union of undefined and the type of the last fragment if there were any nullable keys in the chain (e.g. undefined|string) when using useProperty('security.user.name').

For the first problem, I've come with the following solution:

--- a/packages/vue/src/composables/property.ts	(revision 62d1293548e994cd2e997c6aa64e178526f54b9b)
+++ b/packages/vue/src/composables/property.ts	(revision e5df3f34973aff49e94ca5dd521b565782abc98e)
@@ -41,10 +41,10 @@
 
 type PathImpl<T, K extends keyof T> =
 	K extends string
-		? T[K] extends Record<string, any>
-			? T[K] extends ArrayLike<any>
-				? K | `${K}.${PathImpl<T[K], Exclude<keyof T[K], keyof any[]>>}`
-				: K | `${K}.${PathImpl<T[K], keyof T[K]>}`
+		? NonNullable<T[K]> extends Record<string, any>
+			? NonNullable<T[K]> extends ArrayLike<any>
+				? K | `${K}.${PathImpl<NonNullable<T[K]>, Exclude<keyof NonNullable<T[K]>, keyof any[]>>}`
+				: K | `${K}.${PathImpl<NonNullable<T[K]>, keyof NonNullable<T[K]>>}`
 			: K
 		: never
 

For the second problem, we'll need to find a way to make PathValue return the correct type depending on the chain.

@innocenzi innocenzi added priority: low Nice to have, enhancement and removed pending triage This issue needs a proper label to be attributed labels Mar 6, 2023
@innocenzi innocenzi linked a pull request Mar 6, 2023 that will close this issue
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: low Nice to have, enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants