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

Cannot watch template variables if Form has notEmpty() rules #567

Open
1 of 3 tasks
chinpei215 opened this issue Dec 20, 2017 · 11 comments
Open
1 of 3 tasks

Cannot watch template variables if Form has notEmpty() rules #567

chinpei215 opened this issue Dec 20, 2017 · 11 comments
Assignees
Labels
Milestone

Comments

@chinpei215
Copy link

chinpei215 commented Dec 20, 2017

This is a (multiple allowed):

  • bug

  • enhancement

  • feature-discussion (RFC)

  • CakePHP Version: 3.5.8

  • DebugKit Version: 3.11.3

What you did

Use modelless Form with notEmpty() rule, assign it to template variables, and click the Variables tab of the DebugKit.

What happened

Get Serialization of 'Closure' is not allowed error.

What you expected to happen

Template variables can be watched.

Why this issue happened

notEmpty() uses Closures internally and ToolbarService calls serialize() here.

(Reported by powder_mikan in #japanese cannel on Slack)

@chinpei215 chinpei215 added this to the 3.11.x milestone Dec 20, 2017
@markstory
Copy link
Member

Looks like the __debugInfo of Form is returning closure instances. Perhaps we should have Form or the Validator remove the Closure instances out?

@chinpei215
Copy link
Author

Thank you for your comment. I didn't know DebugKit calls __debugInfo(). Then it would be an idea. But first, let me try to fix this walker. If it ensures that the return value doesn't contain any closures, this kind of issues will no longer happen.

@chinpei215 chinpei215 self-assigned this Dec 25, 2017
@chinpei215
Copy link
Author

It was not so easy 😞 At first I was planning to replace closures with string '(closure)' recursively. But it would break objects in view variables, or runs into infinite loops if I don't take care.

@markstory
Copy link
Member

I wonder if we should try storing data that isn't the serialized variables. I don't know what that would be, but Debugger::exportVar() might be a good place to start or use as inspiration.

@chinpei215
Copy link
Author

If we don't need to use serialize(), then I think the built-in var_export() may be a good function. It can stop when recursion detected.

@markstory
Copy link
Member

Not using serialize() may make expanding variable contents much harder as right now we rely on being able to traverse objects/arrays and generate the nested HTML structures. We wouldn't be able to do that with the output of var_export().

@chinpei215
Copy link
Author

Hmm. It seems to be harder than I thought. Indeed, using var_export() doesn't make sense as we would get Call to undefined method Closure::__set_state() instead of Serialization of 'Closure' is not allowed.

But I think traversing variable like Debugger::exportVar() is not a good idea either, as if the following variable is assigned, DebugKit has to export recursively until max depth:

$array = [];
$array['array'] =& $array;

It would make DebugKit slow.

@markstory
Copy link
Member

We could keep max-depth shallow, like 3 or 4 levels, as a way to prevent slowdowns. I don't think we'd be able to use Debugger::exportVar() as is, because we'd still want a way to create clickable HTML out of the variable dumps.

@markstory markstory modified the milestones: 3.11.x, 3.12.x Jan 24, 2018
@markstory markstory modified the milestones: 3.12.x, 3.x Feb 19, 2018
@chinpei215
Copy link
Author

Oh sorry, I forgot about this issue. Now I am thinking fixing Form::__debugInfo() would be safe, as I am not sure if using Debugger::exportVar() doesn't cause another issues like this.

@othercorey
Copy link
Member

Do we need to create an issue in cakephp?

@LordSimal
Copy link
Contributor

Is this still valid?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants