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

Set controller parameter type of all use functions to Controller #216

Merged
merged 3 commits into from Oct 21, 2022

Conversation

marcoroth
Copy link
Member

@marcoroth marcoroth commented Aug 6, 2022

Before:

When using stimulus-use with TypeScript it complains if you pass in a regular controller into a use function which had the sub-controller as the argument type.

The type for the first argument was not consistent over all the use functions, half of them were Controller (which is the "right" way) and the other half was [Name]ComposableController (or similar).

Since this is just an issue when using TypeScript in the host-application it probably went unnoticed until now.

function useIdle(controller: IdleComposableController, ...) {
  // ...
}

export default class extends Controller {
  connect() {
    useIdle(this)
    // ^^^^^^^^^^
    // Argument of type 'this' is not assignable to parameter of type 'IdleComposableController'.
    // Property 'isIdle' is missing in type 'default' but required in type 'IdleComposableController'.
  }
}

After:

This PR changes all use functions to take a regular Controller as the first argument and casts it to the sub-type in the function body so that the rest of the function can operate on the casted controller.

function useIdle(composableController: Controller, ...) {
  const controller = composableController as IdleComposableController
  // ...
}

export default class extends Controller {
  connect() {
    useIdle(this)
  }
}

I'm not super happy with the name composableController as the first argument name. If you have any better ideas let me know.

Resolves #213

@marcoroth marcoroth marked this pull request as ready for review August 6, 2022 01:27
@gjtorikian
Copy link

@marcoroth Any chance for a merge and release? Thanks!

@marcoroth
Copy link
Member Author

@marcoroth Any chance for a merge and release? Thanks!

Hey @gjtorikian, yes! I want to cut a new release this week, including this PR.

@marcoroth marcoroth merged commit cb6d3c1 into main Oct 21, 2022
@marcoroth marcoroth deleted the fix-composable-types branch October 21, 2022 12:31
@marcoroth
Copy link
Member Author

Released as 0.51.0

@gjtorikian
Copy link

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants