obsidianmd / obsidian-api

Type definitions for the latest Obsidian API.
https://docs.obsidian.md
MIT License
1.65k stars 192 forks source link

Global `app` deprecated in API but is still supported according to release notes #118

Closed pjeby closed 1 month ago

pjeby commented 11 months ago

The global app was made part of the official API last year ( https://obsidian.md/changelog/2022-03-31-desktop-v0.14.4/ ), but a recent commit appears to have marked it deprecated (under the commit comment "Add some descriptions").

Is this change actually intentional? If so, why the reversal in policy (and lack of mention of said reversal in any release notes)?

From prior discussion on a plugin PR, it sounds like it was marked deprecated in Obsidian's internal code base on 2022-09-29 (though I don't know by whom), but then not pushed to Github; it seems someone changed things since that discussion so it would push to Github, but did not update any release notes nor has anyone said if the deprecation was actually intended. (Which would be kind of weird since it had been made public just six months prior to that).

Anyway, I have been using global app in plugin development ever since the official announcement last March, so I'd like to get some clarity on this going forward. Thanks.

lishid commented 11 months ago

Yeah.... so we went back and forth on this in the beginning. You can use it with no problem, and we will not remove it for the foreseeable future. The deprecation notice is there because we want to generally discourage plugin authors to use the global/singleton pattern. If you know what you're doing then the rules don't apply to you 😉 .

pjeby commented 11 months ago

What if you just export app as a property of require("obsidian")? Then it's not a global since you could literally change it for each plugin if you wanted. :wink: Exporting a getApp() function would work too.

Or are you saying you just want to make sure people have to pass the app object around from one object to another? If so, I'm curious as to what benefit you see in that.

lishid commented 10 months ago

Sorry forgot to get back to you on this - I don't have a great answer as to why I don't like the use of global app. Perhaps it's just because it's a practice we don't use internally.

Either way, I generally prefer passing objects around to maintain a general sense of "what knows what" and "what is supposed to know what", OOP style. And I tend to avoid the style where all the functionality is written with top-level functions, especially if it make uses of global app.

pjeby commented 10 months ago

So, does that mean you're opposed to exporting it via the "obsidian" module instead of exposing it as a global?

My question was less about the benefit to the design internal to Obsidian-the-app (of which there are several I can think of, such as making code more unit-testable) and more about why you didn't want plugin developers using that style. (That is, what's the benefit of pushing others to code that way, especially since the design tradeoffs and DX for plugin developers can be very different from those of app developers. For example, you have a single code base so "sharing" components across it is simpler, and you can e.g. hack one part of Obsidian to special case things for another part, .)

LeonardoGentile commented 2 months ago

Hello, any decision or update on this? I second the idea of exporting the app or a function returning that from the obsidian module. I've downloaded the official API types and they lacks so many properties and methods actually accessible and exposed by the global app. I find them very useful (as the access to getPlugin ) and it could be beneficial to developers if you clarify this global app discussion and actually add App types for the unofficial methods and properties (as to make them official 😄 )

joethei commented 2 months ago

The global app object will be removed from the official type definition in an update soon. You will still have access to all the methods by accessing this.app from inside the plugin class, or <pluginInstance>.app

The unofficial methods are not related to this discussion, they are not documented because they are private. There are loads of methods and fields that exist at runtime but aren't in the API because we have not committed to supporting them through the API.

If you decide to use a private API you can do so by adding it in your own local .d.ts file in your plugin repo.

LeonardoGentile commented 2 months ago

@joethei thanks for the clarification. I get we would still access app via this.app from within plugin instances but would it still be exposed as global? From the response I've understood that it will just be removed from the type API definitions, not actually removed from the global, isn't it?

joethei commented 2 months ago

It will still exist, but we really dislike plugins using it.

pjeby commented 2 months ago

Simple workaround for folks who prefer to keep to the LoD as much as possible:

let app: App;

class MyPlugin ... {
    onload() {
        app = this.app;
        // be happy from here
    }
}

Ophidian will also provide an export let app in a future release, since it already has to receive the plugin instance at plugin construction time, so not a big change there. It'll just be a module-level "global" instead of a globalThis/window global.

(The caveat is of course that you can't use the global until the plugin starts, but you shouldn't need to before then anyway.)

joethei commented 1 month ago

Closing since it has been removed in the latest insider