search

scribu / wp-scb-framework

Utilities for speeding up WordPress plugin and theme development
https://github.com/scribu/wp-scb-framework/wiki
220 stars 59 forks source link

Clarify when framework classes can be instantiated and used #19

Closed Rarst closed 11 years ago

Rarst commented 11 years ago

I am updating plugin, using old (1.x) version of scbFramework and naturally some things have changed, but I also have trouble distinguishing what are possibly new requirements vs what wasn't supposed to work in first place.

  1. Plugin uses options and admin page classes, with their setup hooked to init and admin_init (latter actually seems wrong, but hey - that was long time ago and it worked).
  2. This behaves weird with current framework version. I get some things like link to page in plugin list, but page itself permission errors.
  3. All examples for current version seem to show instantiating classes inside callback, passed to scbInit.

So, are classes limited to being instantiated inside the callback now? Or I am just hitting some weird issue because of legacy code?

Obviously with new approach the earliest you can use classes is callback (so code can be loaded), but can they be used out of it / later?

Might make sense to add doing_it_wrong notices to classes that need to run earlier than specific moment in load process.

scribu commented 11 years ago

Unless everything magically works if you move the instantiation into the callback, I'd wager it's some other legacy issue.

So, are classes limited to being instantiated inside the callback now?

No, but it's the safest place, since you don't have to care what add_action() calls of their own they're making.

For example, scbAdminPage currently uses the '_admin_menu' hook, but that might change if/when the admin menu API is revamped.

Rarst commented 11 years ago

Unless everything magically works if you move the instantiation into the callback

Pretty much, for some reason it really didn't like admin_init and other hooks around there I tried. Here is final diff that made it work https://bitbucket.org/Rarst/deny-spam/commits/b371ce5241ac9d4dab85f20eb9e2a204

No, but it's the safest place

Noted, thanks. Maybe should be mentioned as explicit recommendation in wiki?

For example, scbAdminPage currently uses the '_admin_menu' hook

Maybe worth a check and notice if run too late? Like core does in some places - this needs such and such hook, but you are running it too late and they have been passed.

scribu commented 11 years ago

Maybe worth a check and notice if run too late? Like core does in some places - this needs such and such hook, but you are running it too late and they have been passed.

What would be interesting is to have some sort of debugging tool that instruments add_action() so that it always triggers a warning if the action being registered was already fired.

scribu commented 11 years ago

Maybe should be mentioned as explicit recommendation in wiki?

Done: https://github.com/scribu/wp-scb-framework/wiki/Home/_compare/a5fbef7f8118cc2ad7b504e82b9552aa078ec6aa...cbf1e2e5f0e69dd46c232ef1eb51a51196ba43a9

scribu commented 11 years ago

I noticed that scbAdminPage::__construct() has an add_action( 'admin_init' ) call, so that was definitely it.

Rarst commented 11 years ago

What would be interesting is to have some sort of debugging tool that instruments add_action() so that it always triggers a warning if the action being registered was already fired.

Great idea but the only technical way I can think of (so far) would be adding wrapper, which detracts from convenience. I will think about it some more and maybe experiment in my Advanced Hooks API plugin.

Thank you for wiki update, I think that is sufficient to resolve the issue in scope I formulated.