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

Service Dependency Type-Checking #38

Closed
wants to merge 16 commits into from

Conversation

freshgum-bubbles
Copy link
Owner

@freshgum-bubbles freshgum-bubbles commented Jul 3, 2023

BREAKING CHANGE

Introduce type-checking into the Service object, to force
consumers of the Service decorator to correctly utilise the
expected type. This reduces runtime errors, and provides
static type-checking for dependencies passed into Service.

With this fix, the code in #32
no longer compiles. Therefore, this fixes #32.

Please excuse the verbosity in the implementation.
The reasoning for this is described in the TSDoc comments.

This is considered a draft until some form of type-testing is performed
on the overloads. As the implementation is verbose, there are chances
that future changes may break the overloads without the current suite
being able to detect it.

Todo

  • Service type-checking
  • JSService type-checking
  • Type testing against overloads

@freshgum-bubbles freshgum-bubbles added this to the Version 1 milestone Jul 3, 2023
@freshgum-bubbles freshgum-bubbles marked this pull request as draft July 13, 2023 04:27
This utility type allows us to easily type classes
with both an expected instance type, and a set
list of arguments.
While this implementation is very... verbose,
it does work and I see no practical way around it
in the current version of TypeScript.

I've tried to condense the documentation by way
of TSDoc inheritDoc tags, but they don't actually
display in solutions such as VSCode.
This makes the documentation worse, as the docs
for each static overload wouldn't be visible without
scrolling to the generic fall-through overload.

Currently, the "everything is an option object" overload
isn't type-checked.  This makes the code a bit less
verbose, but it may be desirable.
Add support for using built-ins as dependencies
in the type-checking algorithm.
Ensure built-ins are converted to the correct type
when type-checking dependencies.
For instance, String should convert to string.
The code was never meant to make it into
a commit, but oh well.
The new name is more descriptive of what the type does.
Furthermore, simplify the type-resolution algorithm and remove its
dependency on arrays with a fixed number of items.
The new implementation is compatible with arrays of any size.

A major advantage of this implementation is that it **dramatically**
simplifies the implementation of dependency type-checking,
while also removing the need for multiple Service / JSService overloads.
Overall, this means much less code duplication.
This brings the JSService in-line with that of Service.
This makes it more clear what the file does.
This is temporary. I'd like to find a better way.
However, currently TypeScript thinks the type
is excessively deep if we cast it.
@freshgum-bubbles
Copy link
Owner Author

Done a bit of research after seeing compile-time errors in the current implementation.
It looks like we can't actually pass a mapped tuple type as a rest parameter.
They're arrays... but they're not really arrays 🫠
microsoft/TypeScript#29919

@freshgum-bubbles freshgum-bubbles mentioned this pull request Sep 4, 2023
3 tasks
@freshgum-bubbles
Copy link
Owner Author

Superseded by #85.

@freshgum-bubbles freshgum-bubbles deleted the feature/service-type-checking branch September 9, 2023 21:00
@github-actions
Copy link
Contributor

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 10, 2023
@freshgum-bubbles freshgum-bubbles added the enhancement New feature or request label Oct 26, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dependencies aren't currently type-checked
1 participant