puphpeteer
puphpeteer copied to clipboard
PhpDocs update
- Fixed style of generated phpDocs to allow IDE use them.
- Replaced callbacks with JsHandler
- Detailed arguments in traits.
Tested in PHPStorm.
P.S. Docs generation from js code is awesome yet simple to read!
Hello!
Can anybody check this PR and merge or send feedback?
Hi! I've started to write a review, I will try to finish it tonight.
Many thanks! There is no hurry. I just wanted to know, if i need to wait little bit more or this PR will be buried in sands of time :)
To make review simpler, here is the list of files i have changed:
- composer.json
- src/Command/GenerateDocumentationCommand.php
- src/Traits/AliasesEvaluationMethods.php
- src/Traits/AliasesSelectionMethods.php
- src/doc-generator.ts
After that i run docs generator cli command to update automatically generated phpDocs.
P.S. Docs generation from js code is awesome yet simple to read!
I'm glad it helps you!
@nesk Ive reverted Traits changes.
But i still think we need to fix them :) :
Also i've done some stuff:
- Add Puppeteer methods from Puppeteer and PuppeteerNode
- Remove build folder before building docs
- Added JsFunction type instead of callbacks. Also added replacement of mixed to JsFunction for evaluation* methods.
Will wait for your approval and merge ;)
IDE autocompletion issues
I have downloaded PHPStorm and tried to setup everything, I must agree: it doesn't work well. 😅
However, you can clearly see with this PHPStan demo what it brings to have those annotations, they really help. I'm don't want to give up on them, but I have some ideas to fix the current issues.
Here are some interesting points:
- PHPStan works really well with the documentation, as seen above.
-
Psalm doesn't work with the way the
@method
annotations are written, I wasn't expecting that. - PHPStorm doesn't work with the documentation, as you said.
The good news:
- PHPStan can use
@phpstan-method
instead of@method
, which would allow to fully type the methods without breaking Psalm and IDEs. - Psalm can use
@psalm-method
instead of@method
, which would allow to use better typings than the ones for IDEs and not break on PHPStan format. - In addition, we can add simple
@method
annotations which are working everywhere and will allow IDEs to have proper autocompletion.
By using the three documentation formats here, we could achieve perfect support everywhere.
Callback documentation
You are right, the current typings for the callbacks aren't the good ones, because PuPHPeteer expects you to use a JsFunction
instance. You've made the following changes:
- * @method mixed download(string $revision, callable(float $x, float $y): void $progressCallback = null)
+ * @method mixed download(string $revision, \Nesk\Rialto\Data\JsFunction $progressCallback = null)
This is correct, however we lose the detailed typings. I think we can fix this by doing this:
- * @method mixed download(string $revision, callable(float $x, float $y): void $progressCallback = null)
+ * @method mixed download(string $revision, (callable(float, float): void)|JsFunction $progressCallback = null)
This works perfectly and can still help users to see the detailed typings, here's an example.
Additionally, I could add in Rialto a trigger to warn users about the fact they are using a callable
instead of a JsFunction
instance.
FQCN references
What do you think? Do you insist on full namespaces instead of FQCN?
I've tought about it, I see no difference on your side (as a user) if I use FQCN refs or relative ones. As the maintainer of the project, I would rather keep the FQCN references since I find them more reliable.
PHPStorm makes no difference between relative or FQCN references, it always displays the FQCN one:


To conclude
I see immediate benefits in some of your changes:
- You have fixed the missing
symfony/console
dependency. This is already merged in another PR, thank you again! - You have fixed the missing
$
character insrc/Traits/AliasesEvaluationMethods.php
. - You have fixed the missing properties and methods in
src/Puppeteer.php
.
If you want, you can make a new PR with only the last two changes in this list and I will merge it right away. 🙂
About everything else, there is still work to do:
- The documentation generator should output three formats for each property/method/etc to support the majority of the tools, namely PHPStan, Psalm and all other tools reading phpDoc (like IDEs).
- The documentation for callbacks should support
JsFunction
instances without removing the detailed typings, a union can solve this.
If you don't have time anymore to work on these, I can take over. However, I may not have time to work on this soon so you will have to deal with the current typings for a while.
-
IDE autocompletion issues
Good idea to use @method
, @phpstan-method
, @psalm-method
. I will modify doc-genereator. Add some property, to contriol output format of docs. We will run it 3 times, with different formats, and then merge them and write to phpDocs.
-
Callback documentation
Union is ok, i will do that.
-
FQCN references
Ok. Will return full namespace
Thanks for nice ideas, will implement them in upcoming days.
Hi! Sorry for delay with this fixes.
Fixed: FQCN references and callbacks unions (for phpstan format). Added support for multiple doc styles for php.
But I ve run into another issue: phpStorm don't support @phpstan-method with our annotations. And it complains about duplicate methods:
also hints are incorect:
My proposal: replace @phpstan-method
with non annotation string, like @method-extended
. This is only way i can manage phpStorm works:
@nesk Hi! Want to remind about my pull request :)
Hi! PhpStorm got update with generics support in PHPDocs. Maybe this will help to cleanup this PR.
Hi! PhpStorm got update with generics support in PHPDocs. Maybe this will help to cleanup this PR.
Checked in new PhpStorm and there is still no support for array shape for argement types in @method
. So PR will be same.