-
Notifications
You must be signed in to change notification settings - Fork 10
Project Setup #7
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Coool! Ca marche bien! Merci Maelan 🧡
src/TypeScriptBuilder.php
Outdated
{ | ||
$files = []; | ||
$dirs = []; | ||
foreach ($this->typeScriptFilesPaths as $typeScriptFilePath) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Si on utilise le bundle et qu'on a plusieurs niveau de dossier, on a une erreur: file_get_contents failed to open stream. Je pense que c'est important qu'on puisse avoir autant de niveau de dossier qu'on veut dans notre dossier typescript.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tu peux me donner plus d'info sur ta config et ton arbo stp ? je n'arrive pas à reproduire le bug
905bf5d
to
40786ea
Compare
40786ea
to
5499689
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks exactly how I hoped. I didn't try it yet though... does it work? :)
parent::__construct(); | ||
} | ||
|
||
public function execute(InputInterface $input, OutputInterface $output): int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this support a --watch
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I opened an issue about this one. The compiler supports a watch
option but it requires a node module, and even with the module installed it doesn't seem to work when running swc from the binary. So for now I'd say no, but maybe a Symfony FileWatcher component (or part of the FileSystem component) could solve this ;)
One thing I forgot! In 6.4, there is a new event - https://github.com/symfony/symfony/blob/6.4/src/Symfony/Component/AssetMapper/Command/AssetMapperCompileCommand.php#L65 The idea was to make bundles like this hook into that event, and run their build process. This will prevent users from needing to manually run |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should be closed now! After you addressed the comments we can release the first version 😁
|
||
if (!$process->isSuccessful()) { | ||
$output->error('TypeScript build failed'); | ||
throw new \Exception('TypeScript build failed'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
throw new \Exception('TypeScript build failed'); | |
throw new \Exception(sprintf('Error compiling TypeScript: "%s"', $process->getErrorOutput())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is necessary since we output the buffer of the process while it's running. We would end up having the error message displayed twice
@weaverryan @WebMamba Thanks a lot for your reviews. I'll close this but feel free to notify me if there's anything else you'd like to see in the bundle or if you find any problem. 🙏 |
No description provided.