tempestphp / tempest-framework

The PHP framework that gets out of your way 🌊
https://tempestphp.com
MIT License
989 stars 71 forks source link

Discovery cache improvements #395

Open brendt opened 1 month ago

brendt commented 1 month ago

Let's look into whether we can improve discovery cache for local development

aidan-casey commented 1 month ago

@brendt - I have some thoughts here...

While less developer friendly, have you tried parsing files with nikic/php-parser at all? I'd imagine it'd be faster, but unsure. We'd also avoid some runtime issues we have today (e.g., if you run discovery across all files in the vendor dir, you might load a Symfony class relying on a dependency not installed, etc.).

I'm wondering if we could build a few helpers over top with some basic AST checks (e.g., Attributes, whether a class extends another, whether a class implements another, methods, parameters, etc.).

We might be able to also use this method to pull caching out of the hands of individual discoverers and only handle it at a discovery manager level.

brendt commented 1 month ago

While less developer friendly

😱

have you tried parsing files with nikic/php-parser at all? I'd imagine it'd be faster, but unsure

Why? Compared to reflection? Noooo definitely not, reflection is super fast

if you run discovery across all files in the vendor dir, you might load a Symfony class relying on a dependency not installed,

We shouldn't though? We're only scanning Tempest packages. On top of that, we're already catching runtime exceptions and are simply not loading those classes

I'm wondering if we could build a few helpers over top with some basic AST checks (e.g., Attributes, whether a class extends another, whether a class implements another, methods, parameters, etc.).

We have this with the reflector helpers — I find they really useful, you don't?

We might be able to also use this method to pull caching out of the hands of individual discoverers and only handle it at a discovery manager level.

That is the goal, but I don't see why we'd need to switch reflection for it?

Wulfheart commented 1 month ago

We shouldn't though? We're only scanning Tempest packages. On top of that, we're already catching runtime exceptions and are simply not loading those classes

But what exactly is a tempest package? Can I make m own tempest package which also gets autodiscovered?

brendt commented 1 month ago

@Wulfheart every package that has a dependency on a tempest core package is considered a tempest package. Right now you'd need to require eg. tempest/core and your package will be discovered. However, I plan on adding some kind of "package tools package" that all third party packages should require.