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

Ability to extend the Stimulus Application class #586

Closed
lb- opened this issue Oct 3, 2022 · 5 comments · Fixed by #603
Closed

Ability to extend the Stimulus Application class #586

lb- opened this issue Oct 3, 2022 · 5 comments · Fixed by #603

Comments

@lb-
Copy link
Contributor

lb- commented Oct 3, 2022

Summary

Class inheritance provides a way to achieve custom behaviour and it would be great to also allow this at the Application level.

Currently, it is not easy to extend and use the Application class without rewriting its static start method.

Details

In the Application code, the start static method references the Application class directly instead of using this.

static start(element?: Element, schema?: Schema): Application {
const application = new Application(element, schema)
application.start()
return application
}

This means that even if you extend the Application with say a custom method override of load, this custom method will never get used.

Proposal

Using this instead of Application should solve the issue.

  static start(element?: Element, schema?: Schema): Application {
    const application = new this(element, schema)
    application.start()
    return application
  }

Use case

A trivial use case to add extra behaviour to the application stop call.

class MyApplication extends Application {
  stop() {
    super();
    console.log('application stopped');
  }
}

const application= MyApplication.start();
application.stop();
@marcoroth
Copy link
Member

Hey @lb-, thanks for opening this issue.

I think your proposal makes sense and we could implement this. Feel free to open a pull request for it! Thank you!

@lb-
Copy link
Contributor Author

lb- commented Oct 4, 2022

Thanks @marcoroth - should this be documented anywhere? Or is it best left undocumented?

I would assume we would add a section around the application debugging or before the application error callback section?

https://stimulus.hotwired.dev/handbook/installing#debugging

@marcoroth
Copy link
Member

I would probably have left it undocumented, since it's technically "just" regular class inheritance at work. But I wouldn't be opposed to having a small note/example in the documentation for it.

@lb-
Copy link
Contributor Author

lb- commented Nov 2, 2022

Thanks. I still have this on my radar and hope to get a PR up when I get the chance.

lb- added a commit to lb-/stimulus that referenced this issue Nov 17, 2022
When using const application = MyApplication.start() - it now will use the actual MyApplication class instead of the non-extended Application class
Fixes hotwired#586
lb- added a commit to lb-/stimulus that referenced this issue Nov 17, 2022
- When using const application = MyApplication.start() - it now will use the actual MyApplication class instead of the non-extended Application class
- Fixes hotwired#586
@lb-
Copy link
Contributor Author

lb- commented Nov 17, 2022

PR up #603

@dhh dhh closed this as completed in #603 Nov 18, 2022
dhh pushed a commit that referenced this issue Nov 18, 2022
…#603)

- When using const application = MyApplication.start() - it now will use the actual MyApplication class instead of the non-extended Application class
- Fixes #586
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants