search

yiisoft / yii2

Yii 2: The Fast, Secure and Professional PHP Framework
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
14.23k stars 6.91k forks source link

registerJs, registerCss (etc) after View::endBody() #18328

Closed atiline closed 2 years ago

atiline commented 3 years ago

Hi!

I suggest throwing an exception or calling \Yii::warning or \Yii::error when functions yii\web\View::registerJs, yii\web\View::registerCss etc are called after function yii\web\View::endBody() was called.

Now there are any checks of Application or View states.

Some example - you can call registerJs() after Application::EVENT_AFTER_REQUEST. Any JS will be added to response.

samdark commented 3 years ago

Sounds alright. But may I wonder how did you experience that?

atiline commented 3 years ago

If my request contains some "hard" logic (many db queries, some http requests to another hosts etc) I use some trick - on Application::EVENT_AFTER_REQUEST I call another php process (passing args via cache or db) and run my logic there (in console application).

Sometimes I forget that on this application state I can't use registering assets and it's really hard to debug why JS files were not atached. Some warning will be useful.

lubosdz commented 3 years ago

Formarly sounds reasonable but applies limits to those rare justified cases I've seen around. So this would be either an error or an intention of a developer - in first case framework should not apply BC breaking limits, in second case framework should not attempt to solve developers mistake. It's edge case.

samdark commented 3 years ago

I wonder if registering anything after request could be a valid use-case...

@yiisoft/reviewers, @yiisoft/core-developers, @yiisoft/yii2 what do you think?

bizley commented 3 years ago

I agree with @lubosdz - even though it is a valid issue it's not common and should be resolved by the developer.

My6UoT9 commented 3 years ago

I think the wish of the OP is not to support this behaviour, but to throw some warning when this happens. Just throwing a notice should not be a BC?

bizley commented 3 years ago

Yes, warning could be added, indeed.

machour commented 2 years ago

The merged PR seems to trigger errors using the app basic template, see https://github.com/yiisoft/yii2/pull/18972#issuecomment-950700577

To be investigated / fixed before next yii2 release.

samdark commented 2 years ago

See https://github.com/yiisoft/yii2/pull/18972#issuecomment-950723220

perlexed commented 2 years ago

Comment https://github.com/yiisoft/yii2/pull/18972#issuecomment-950723220 has the correct assumption. Pull request https://github.com/yiisoft/yii2/pull/18975 implements the suggestion from this comment