brefphp / bref

Serverless PHP on AWS Lambda
https://bref.sh
MIT License
3.05k stars 364 forks source link

fix: add missing chalk dependency #1831

Closed czubocha closed 6 days ago

czubocha commented 1 week ago

chalk is no longer a dependency for serverless v4. bref must install chalk on its own to use it.

mnapoli commented 1 week ago

Hi, thanks for the PR!

As the tests show, this cannot work here: Bref is distributed via Packagist (equivalent of npmjs.com for PHP). The dependencies in package.json are not installed, which explains why the plugin has to do so many weird things to work.

And it cannot be externalized into a NPM package because the goal is to have a one-step experience for PHP developers (i.e. they don't have to install Bref via Composer and NPM).

Is chalk the only problematic dependency? If that's the case, we probably can adapt things here.

If not, then maybe supporting Serverless v4 needs to happen in a NPM-installable plugin, but the experience for v4 would be worse than v3 🤔

czubocha commented 1 week ago

Hey! As far as I’m concerned, chalk is the only problematic dependency. I pushed a change to the PR to remove chalk and use the new logger method available in v4 that prints messages in grey. This way, we keep the grey color in v4 without breaking v3 (though the message will be in the default color in v3). Let me know what you think.

mnapoli commented 6 days ago

Replaced by #1837

Thanks!