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

Roadmap v5.0 #88

Closed
5 tasks done
WyriHaximus opened this issue Jun 22, 2018 · 31 comments
Closed
5 tasks done

Roadmap v5.0 #88

WyriHaximus opened this issue Jun 22, 2018 · 31 comments

Comments

@WyriHaximus
Copy link
Collaborator

WyriHaximus commented Jun 22, 2018

Todo:

  • Twig 3.0
  • Bump minimum PHP version to 7.1
  • Merge in all changes from the 4.x branch
  • Clean up and reorganize filters and functions

Research:

  • Disable autoescaping for functions and filters so |raw everywhere isn't required (via @markstory)
@inoas
Copy link
Contributor

inoas commented Jun 22, 2018

Targeting CakePHP 4.0 (or 3.7) only?

@WyriHaximus
Copy link
Collaborator Author

Probably CakePHP 4.0, but if 3.7 isn't to much of an hassle that would be awesome. The thing is I would prefer to support the same minimum PHP version as CakePHP 4.0 with this 5.0 release.

@WyriHaximus
Copy link
Collaborator Author

@ADmad && @markstory Any things on cake's wishlist for 5.0 (targeting cakephp 4.0)?

@markstory
Copy link
Member

I would love to help find a way to not need |raw all over the place. I find it noisy and trains people to use |raw which can be dangerous.

@WyriHaximus
Copy link
Collaborator Author

@markstory looking into how to sanely do that 👍

@ADmad
Copy link
Member

ADmad commented Jul 24, 2018

Just skimming through twig docs I couldn't find a way to tell twig to disable escape for specific vars used in {{ }}.

So avoiding the need for using |raw would probably require registering new tag for helpers as it's done for cells and element. Usage would be like {% helper 'Foo.method' %}. This would make the usage a bit more verbose but on the flip side it would provide the benefit of being able to use helper auto loading like we have for .ctp files. Currently you need to explicitly load the helpers you want to use in twig template.

@WyriHaximus
Copy link
Collaborator Author

There might be something deeper in the API to do this, not entirely sure.

Like the idea, not 100% sold on the syntax but I'll get an experiment on this weekend to implement it and see where we can take it from there 👍 .

@markstory
Copy link
Member

@ADmad TwigView could use reflection to generate twig functions for all defined helper methods. Twig functions can indicate that they are escapers. This would add a bit of overhead but could be something that is 'compiled' or cached.

@ADmad
Copy link
Member

ADmad commented Jul 26, 2018

@markstory Yeah using reflection in TwigView is an option but I would prefer if the additional work could be done through Twig itself (with some additional code) instead of adding reflection in our view class.

On a related note could be shorten the tag for element to elem? :)

@ADmad
Copy link
Member

ADmad commented Jul 26, 2018

Having helpers as regular variables also has the drawback that if someone doesn't follow conventions and for e.g. sets a view var named Html, the variable will be overwritten by helper instance.

@WyriHaximus
Copy link
Collaborator Author

On a related note could be shorten the tag for element to elem? :)

Sure, could probably do both 😜

@WyriHaximus
Copy link
Collaborator Author

@ADmad just been digging around and found this: https://twig.symfony.com/doc/2.x/advanced.html#automatic-escaping
We could/should select specific functions and filters that will be safe to be output rawly into the resulting template

@markstory
Copy link
Member

@WyriHaximus Yes, that would be really useful on Helper proxy functions.

@WyriHaximus
Copy link
Collaborator Author

Merging 4.x in master now:

image

Will keep you updated, and tag you in relevant PR's for the extra tags and escaping

@ADmad
Copy link
Member

ADmad commented Sep 29, 2018

I was looking into updating master to run with CakePHP 4 and got stuck. There's a bit of chicken and egg problem as Bake requires TwigView and TwigView itself has Bake as dev dependency 🙂

I tried to work on TwigView first ignoring the Bake dependency but the 1st blocker is the api-clients/test-utilities. Cake 4 requires PHP 7.1+ and phpunit 7.0+ but there's no version of test-utilities which satisfies these dependencies.

@WyriHaximus
Copy link
Collaborator Author

@ADmad lets fix that! If api-clients/test-utilities blocks something we can drop it and required-dev dependencies directly. Mainly use it as convenience to include test utilities but it isn't blocking anything. Is the PHP 7.1 for CakePHP 4 final? Or might it go to 7.2 before 4.0.0 is released?

Also we could make bake an optional dev dependency that we load on the CI when running tests but isn't a dev requirement if that untangles things.

@ADmad
Copy link
Member

ADmad commented Sep 29, 2018

Is the PHP 7.1 for CakePHP 4 final?

It's final.

@WyriHaximus
Copy link
Collaborator Author

Is the PHP 7.1 for CakePHP 4 final?

It's final.

Ok cool! How far from releasing are you?

@ADmad
Copy link
Member

ADmad commented Sep 29, 2018

As always "When it's ready" :)

@WyriHaximus
Copy link
Collaborator Author

As always "When it's ready" :)

Cool, the underlying question was: How fast does this needs to be done. But I'll get onto some stuff after my Honeymoon and dedicate some days to straightening things out and get them CakePHP 4 ready 👍 .

@ADmad
Copy link
Member

ADmad commented Sep 30, 2018

No worries. Congrats on your wedding and enjoy your honeymoon 🙂

@ADmad ADmad mentioned this issue Sep 30, 2018
@WyriHaximus
Copy link
Collaborator Author

Thank you, enjoying it a lot 😎 !

@FinlayDaG33k
Copy link

I've been taking a look at this repo lately because we are considering to implement this in our project, however, I feel like it's "odd" to have to rename all the .ctp files to .twig files to use TwigView.
I wonder if it would be possible to use the regular .ctp files instead?

@ADmad
Copy link
Member

ADmad commented Jun 30, 2019

@FinlayDaG33k "regular" .ctp files contain PHP code :). Why would you unnecessarily confuse any new dev joining your project by using .ctp extension instead of the default .twig for Twig files?

@WyriHaximus
Copy link
Collaborator Author

It's not only that, TwigView will fallback to .ctp files if it can't find the matching .twig file. So both formats are supported and you can iterate your way into support it file by file. There are also some IDE benefits such as PHPStorm picking up it's a twig file without having to configure that.

@FinlayDaG33k
Copy link

FinlayDaG33k commented Jun 30, 2019

Why would you unnecessarily confuse any new dev joining your project by using .ctp extension instead of the default .twig for Twig files?

Well, to me, it feels more confusing that everything is .ctp and then suddenly there are some .twig files hanging around.

It's not only that, TwigView will fallback to .ctp files if it can't find the matching .twig file.
Ah I see.

This wasn't in the documentation, I'll add that in a bit :)

@WyriHaximus
Copy link
Collaborator Author

It's not only that, TwigView will fallback to .ctp files if it can't find the matching .twig file.
Ah I see.

This wasn't in the documentation, I'll add that in a bit :)

Cool 👍 !

@dereuromark
Copy link
Member

the files should be whatever they are. so if those are twig templates, then the file extension should be .twig.
this also makes it able to leverage IDEs here to introspect them more easily.

As for the new Cake4 release, any plans on releasing a beta already or sth?
https://github.com/cakephp/bake/blob/master/composer.json#L24

"wyrihaximus/twig-view": "dev-master"

this is blocking other plugins to become stable.

cakephp/bake 2.0.1 requires wyrihaximus/twig-view dev-master -> satisfiable by wyrihaximus/twig-view[dev-master] but these conflict with your requirements or minimum-stability.

@dereuromark
Copy link
Member

PS: Once you got a beta tag out, like the other plugins now, you can add the strawberry: FriendsOfCake/awesome-cakephp#334 :)

@WyriHaximus
Copy link
Collaborator Author

@dereuromark Hopefully this weekend. Will let you know once the tag is out

@WyriHaximus
Copy link
Collaborator Author

5.0 is out 🎉

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

No branches or pull requests

6 participants