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

[StimulusBundle] Add support for typescript controllers #1335

Merged
merged 1 commit into from
Dec 18, 2023

Conversation

evertharmeling
Copy link
Contributor

Q A
Bug fix? no
New feature? yes
Issues n/a
License MIT

To be able to use typescript-controllers we need to be able to load those controllers. This PR adds support for loading *.ts files.

Full support for typescript-controllers needs the sensiolabs/typescript-bundle to compile the controllers.

A reproducer project can be found here. It needs this PR to run.

Also see this slack conversation.

@weaverryan
Copy link
Member

Good stuff - thank you @evertharmeling!

@weaverryan weaverryan merged commit 1d18a24 into symfony:2.x Dec 18, 2023
26 of 34 checks passed
@@ -1,5 +1,9 @@
# CHANGELOG

## 2.x

- Added Typescript controllers support
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there should be doc somewhere explaining a bit more this change.. as it could be disappointing for some :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point - just a bit more clarity on how .ts controller files are now located (if you use TypeScript bundle to process in the first place)

Copy link
Contributor Author

@evertharmeling evertharmeling Dec 19, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah fair enough, I'll try to add some documentation

EDIT: #1345

@@ -1,5 +1,6 @@
.php-cs-fixer.cache
.phpunit.result.cache
composer.lock
var/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If not necessary, could this be reverted ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd added it because when running the testsuite it created a failure html-file in var/browser/source and to not accidentally add it git. I'll create a PR.

weaverryan added a commit that referenced this pull request Jan 9, 2024
…armeling, weaverryan)

This PR was merged into the 2.x branch.

Discussion
----------

[StimulusBundle] Added docs on TypeScript support

| Q             | A
| ------------- | ---
| Bug fix?      | no
| New feature?  | no <!-- please update src/**/CHANGELOG.md files -->
| Issues        | #1335 <!-- prefix each issue number with "Fix #", no need to create an issue if none exist, explain below instead -->
| License       | MIT

Follow-up on #1335, added more info on usage of TypeScript controllers support.

Commits
-------

237164f Tweaking wording
60c0b8e [StimulusBundle] Added docs about Typescript controller usage
3724401 [StimulusBundle] Revert adding var/ to .gitignore
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.

3 participants