So with the API versioning, I had another thought come to mind and bear with me. How much more difficult would it be to extend that further to let Plugins denote what version(s) of Discourse they are compatible with?
The idea being, when Discourse sees they are not compatible, it can choose to not load them and/or, execute a call on the plugin, so the plugin can disable anything it needs to to permit the Discourse instance from failing and showing a blank screen.
It is the version of the API you coded against, not Discourse. This distinction allows Discourse to provide an API object that is backwards compatible. For example, you might be on a much later version of Discourse, but it can say, oh you wanted version 0.2 of the plugin API? I can build an object that responds the same way even though my internal APIs have changed.
What were you thinking? api.siteSettings?
Not exactly. It’s for client side plugins, that are written in Javascript. For example if you wrote a server side auth plugin, it won’t be affected. Having said that, most client side plugins have an initialize function. If not, how would stuff be wired up?
All the deprecations will show up in the dev console. I’m actually not sure how the Ember console works but I believe it’s meant only for Ember deprecations so they shouldn’t show up there.
I just tried your snippet and it still seems to work on the latest build of Discourse. Long term I do want to allow some customizations built that way, but I have to get around to it.
It’s only topics for now. There are some other parts of Discourse that render slowly that we might visit later, but there’s still a lot of work to do before that happens. My feeling is writing widgets for the virtual dom is quite a bit more complicated so you have to really want the performance benefit to take the tradeoff.
This is trickier than it sounds. For example, let’s say a plugin says it supports 1.3 of Discourse. Is it assumed that it will work with any later version? Because down the road we will certainly deprecate and change some APIs as we build new features.
It’s good for the lower versions though – if you say it requires 1.3 it just wouldn’t load at all on 1.2.
Right. But, if the only change I had to make to my plugin was change the versions support from 1.3 to 1.3-1.4 (pick your poison for how to denote multiple), then I think that is okay. It still puts Discourse in a “safe” mode when it comes to plugins, only load it when it says it is supported, thus hopefully stopping any failures that lead to blank pages.
I’m not saying this is a perfect way of doing it, nor am I saying it should be done soon. Just something I seem to keep coming back to when changes are found that break a good portion of all plugins (though this vdom was a bit forgiving – although I did have one plugin that broke completely due to refactorings in core).
Sounds good. Again, if you thinks it’s not worth exposing. It’s reasonable as the same reason you talk about the api version. The site settings may change.
But the thing actually I want to touch is the plugin enable settings. That’s something api.pluginSettings seems suitable…
What API library can’t give to me is the ability to monkey patch. For sure, Discourse may have more decorators and hook point. However, the change takes 2 years at least until plugin authors enjoy the rich hook point.
As you said, it’s tricky. I hope there’s a better way to handle monkey patching
Some reflections when deploy a change to my plugin:
Before the vdom ( speed, for the effort), I monkey patch components to filter something in the model. Now I decorate it. Some components is widget now because of vdom.
api.decorateWidget can apply before and after type to the decorator. Without applying type, is it applying both?
api.decorateCooked is a little bit tricky. model may exists in stream, but may not appear when composer appear. It’s useful for distinguish the post stream and composer.
I still monkey patch model e.g. topic details; also controller, e.g composer.
The monkey patch have to be inside initializer.
So sorry I can’t demonstrate by code since it’s a private plugin
I initially futzed around with trying to reproduce this logic exactly in the decorateWidget hook, i.e. getting the d-rating component, set its properties and then turning it into html. But this both didn’t work and felt wrong.
Sorry for the reply below with many quotes, but there’s a lot to respond to!
To be honest I’ve never been fully comfortable supporting that kind of snippet, as it’s not importing our modules the correct way and depends highly on our internals. Having said that, I will try my best not to break it until we can offer something that solves the use case.
There should probably be some way of auto detecting the plugin being disabled. I haven’t had to face this case myself but would be open to PRs or suggestions for how to do this.
Well I don’t think it takes us two years to add hook points! Generally we’ll add them very quickly once people ask for them and provide a use case. For reference, our discourse-tagging plugin is barely two years old!
Monkey patching is always risky. We do it in some plugins but the long term goal is to create APIs that handle most of the things people need to patch in. Having said that, if you are adding a new computed property to topic or post, chances are that will work forever.
No, nothing will happen if there is no :after or :before present.
Rather than think of whether you’re in the composer or the post stream, you should just account for sometimes having a model or not. For example, the user stream also applies decorators.
Yes I agree that’s a bit funky. In this case, showRating is a property on the controller right? Is it there because people can enable/disable it on the topic?
The reason I ask is because it’s much more complicated to add a topic controller attribute into the post stream without modifying the handlebars template topic.hbs, but if it’s an attribute on the topic model itself it would be pretty straightforward to add it.
Also in your code, I would recommend using includePostAttributes to add rating to attrs rather than calling getModel. I’d also recommend creating a new widget for the rating, but I haven’t written up how to do that yet because I’ve been quite busy – rawHTML will work but the widget would be nicer and faster.
I could just check whether the post’s rating property has a value and show it if it does, however I will need to provide for hiding ratings in topics that already have them (see e.g.)
However, looking now at @joebuhlig’s nice Feature Voting plugin, I think it would be better if I adopted the approach he’s taken for a similar case - i.e. put this logic in the topic view serializer on the server.
Also in your code, I would recommend using includePostAttributes to add rating to attrs rather than calling getModel. I’d also recommend creating a new widget for the rating, but I haven’t written up how to do that yet because I’ve been quite busy – rawHTML will work but the widget would be nicer and faster
Do you mean beta or stable? As master should have plugin-api. As it is a branch higher than tests-passed. Both beta and master are below, so they wouldn’t have it.
To answer your question, I haven’t toyed with that aspect of it yet. @eviltrout, is there a concern here? As if the lib doesn’t exist in beta and stable and it throws an error due to its non-existence, then this backwards compatibility isn’t truly backwards compatible…
I could have sworn I did a git fetch upstream but I must have done so on a different copy of Discourse than I thought I had.
EDIT
I really should get a new eyeglass prescription or at least put more effort into reading what I can barely see.
Looks like I probably missed the “Gemfile.lock changed commit … Aborted”
(though I didn’t change it, something obviously did)
I’m good now
import { withPluginApi } from 'discourse/lib/plugin-api';
function oldCode() {
// migrate your old plugin code here. It will only be run if no PluginAPI is present
console.log("in old code initializer block");
}
function initializePlugin(api) {
// do stuff with plugin API!
console.log("in plugin API initializer block");
}
export default {
name: 'plugin-outlet-locations',
initialize() {
withPluginApi('0.1', api => initializePlugin(api), { noApi: () => oldCode() });
}
}
I backported withPluginApi to beta and stable, so as long as you are on the latest version of any branch, you will have the function and will never need to conditionally load it. Note that the accepted way to install plugins always involved pulling from the latest branch you are tracking, so updating or adding any plugin will retrieve the withPluginApi support at the same time if you don’t have it.
The only case where you wouldn’t have it is if you are locked to an old commit hash for some reason. I would not recommend that setup.
It may well be that there are no versions of Discourse for which noApi support is needed anymore, so this might be irrelevant, but I can’t seem to work out how to import a module which doesn’t exist in later versions of Discourse, in order to provide backwards compatibility, without breaking the plugin in those later versions.
The completely expected error I get (in the later versions) is:
Error: Could not find module `discourse/components/hamburger-menu` imported from `discourse/plugins/static-pages/discourse/initializers/static-pages-menu`
When trying:
import HamburgerMenuComponent from 'discourse/components/hamburger-menu';
As far as I’m aware, conditional imports aren’t possible in ES6, are they?
You unfortunately can’t do a proper ES6 import conditionally. However, if it’s for your noAPI section, I don’t consider it awful to do a manual style import: