Skip to content

Testing changes #20

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Testing changes #20

wants to merge 1 commit into from

Conversation

djfhe
Copy link
Owner

@djfhe djfhe commented May 4, 2025

Collection of changes i had to implement get this lib to work with a demo project.

  1. While a record might be the correct type when transforming to ts, in practice, nearly everything was a record then. The current change is super dirty and would be better implemented by a "is list" check. But not sure, if the approach is compatible then with 3rd party libraries like LaravelData (thinking about the return type of ::collect() here).

  2. ::collect() returned its whole union of return types (e.g. array|DataCollection|PaginatedDataCollection|CursorPaginatedDataCollection|Enumerable|AbstractPaginator|PaginatorContract|AbstractCursorPaginator|CursorPaginatorContract|LazyCollection|Collection) necessitating the deletion of the count checks. Not sure how we can improve this.

  3. The argument of LiteralTypescriptType Attributes has to be evaluated explicitly since its argument is a string and not a type. If evaluating it via ::transform() the return type would be the type of a constant string with the argument value. But the argument value itself is needed as a type. Therefor we handle it explicitly. PHPstan should be able to analyze it statically most of the time, since the attribute arguments needs to be literal or a constant expression (see https://www.php.net/manual/en/language.attributes.syntax.php).

@djfhe djfhe changed the title Changes for demo project Testing changes May 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant