ophidian-lib / core

A Component Framework for Obsidian Plugins
ISC License
20 stars 4 forks source link

Examples for SettingsService #1

Open sytone opened 1 year ago

sytone commented 1 year ago

Do you have a simple example of how this looks with a Settings Tab class?

Sing the tab will already extend PluginSettingTab it cannot be a service, how will it get context to pull the SettingsService out from the container?

pjeby commented 1 year ago

The idea is that different services have their own settings which together form the overall plugin settings, and the same for the tabs. Each service performs its own useSettings() call and gets the same SettingsService, but the defaults and types from each are combined.

For the settings UI, each service does useSettingsTab() and adds a showSettings() method to actually create the settings UI via the builder.

So there is no PluginSettingTab - that's handled by the builder service (SettingsTabBuilder). An example from some work in progress for one of my plugins:

class GlobalHotkeys extends Service {
    settingsTab   = useSettingsTab(this);
    settings      = useSettings(
        this,
        {globalHotkeys: [] as CmdCfg[]},
        ({globalHotkeys: g}) => this.updateHotkeys(g)
    );
    showSettings() {
        const tab = this.settingsTab, {settings} = this;
        tab.field().setName("Global Hotkeys").setHeading();
        const addNew = tab.field()
            .setName("Add new Command").addButton(b => b
                .setButtonText("Add command").setCta().onClick(async () => {
                    // ...
                    await settings.update(({globalHotkeys: g}) => {
                        g.push(cmd);
                    })
                    new CommandField(this, cmd, addNew.settingEl);
                })
            );
        this.globalHotkeys.forEach(c => new CommandField(this, c, addNew.settingEl));
        tab.field();
    }
}

Basically, there is no one place where you define all your plugins or settings tab UI - it's distributed among the services. The order in which the services are initialized determines the order of the settings in the plugin's (auto-generated) settings tab.

This allows you to concentrate everything for a given feature in one place.

Since the plugin itself is also effectively a service, you can also have it define defaults and UI via the same method; the placement in the list will be determined by where useSettingsTab occurs in the setup, e.g.:

export default class MyPlugin extends Plugin {
    use = use.plugin(this);
    someService = this.use(SomeService);
    settingsTab = useSettingsTab(this, ...);
    otherService = this.use(OtherService);
}

In this example, any settings defined by SomeService will appear first in the plugin's settings, followed by those defined by the plugin class' showSettings() method, followed by any defined by OtherService. So you can organize the settings groups any way you like, and in effect the PluginSettingTab gets generated for you.

pjeby commented 1 year ago

By the way, in case it's not clear from the above, there is a single namespace for the plugin settings, but it's the intersection of all the individual types defined by the services. So service A might use {serviceASetting1, serviceASetting2}, and service B might have {globalHotKeys}. As long as the names don't overlap (or if they do, are a shared setting), it's fine - all the settings are still saved in the normal way, as a combined {serviceASetting1, serviceASetting2, globalHotkeys} type (with combined defaults, too).

But this way, each service only needs to know about its own types and defaults, without needing to manually combine them into a single set of types and defaults for the plugin as a whole.

pjeby commented 1 year ago

Oh, also... to answer your original question, a class does not have to extend Service (or use use.service) in order to be able to look things up. It just needs a use context, which you can pass into its constructor. Or, it can capture use.this if it has a zero-argument constructor, e.g.:

class MyPlugin extends Plugin {
    use = use.plugin(this);
    onload() {
        this.addSettingTab(this.use(MyPluginSettings));
    }
}

class MyPluginSettings extends PluginSettingTab {
    settings = use(SomeService)
    constructor() {
        super(use(App), use(MyPlugin));
    }         
}

Basically, any object that is created via a use() call can call use() to get other services. If that object needs to call it after the constructor is finished, it needs to save it via this.use = use.this (or use = use.this in the class body).

The only reason that Plugin and Services are different on this point is that 1) Plugin is constructed by Obsidian, so there isn't a use() call active (and there is special setup to be done as well), and 2) Services are Components that get registered to load() and unload() with the plugin.

So anything that isn't a component that loads/unloads with the plugin does not have to be a Service. You can also use() anything that has a zero-argument constructor, and it can call use() in the constructor (and any initializers), and it can save use.this (via constructor or initializer) to keep a reference to the context.

sytone commented 1 year ago

Thanks for the detailed response! I will re-read it again later today and look at how I have my plugin structured. Currently my settings are generated by a json configuration file that is processed to generate the UI dynamically. I will look to see how to break it up as the UI creation has a lot of repedative code that I was trying to avoid.

I should be able to make the generation seperate and then have a json file per service to keep it cleaner.

If you want I will make a PR in the next week or so to make this part of the documentation for this library.

I had tried to implement a DI in the past to break up my app but I just do not have enough working experience with typescript to be effective so this project is great as it gets me moving faster. Just have to refactor my code to the service model a bit. I'm sure there would be better ways but I have the rendering working locally now using your framework. Took a bit to sort out how to get the context though for the post code block rendering.

I'll make a PR for my plugin so you can see and comment on my mistakes if you are interested. Thank you for putting all this together.

pjeby commented 1 year ago

Don't worry about the doc PR - the settings API (both the storage and UI parts) are too unstable at the moment, as I have not actually published any plugin updates using them. I'm still exploring the use of them to see if the APIs are actually good. At this point they are functional but there are little bits I don't like, like relying on events for settings updates. The final API probably won't change a ton, but... I don't feel it's stable enough to document at the moment.

You can ping me on your plugin PR, though, and I'll have a look.

I'm going to leave this issue open though as a reminder for me to write some documentation from this eventually. :wink:

pjeby commented 1 year ago

Oh, now I remembered one of the specific things that's most likely to change when useSettings/SettingsService finalizes API is that I may want to require settings to be immutable. The current implementation is based on JSON comparison of the overall settings object rather than equality checking, which is super convenient in some ways but bad for others (e.g. if I rip out Events as the basis of distribution). This is relevant to the API in that I would like to be able to convert the settings "data source" to be something you can functionally derive parts of it as properties, as in FRP systems.

What I mean is, it should be possible to take a settings object, give it two functions (an accessor and mutator for some aspect of settings) and then get a new "subsetting" with the same ability to observe changes and the like. This would make complex settings UI easier if you have settings that need sub-dialogs to pop up with more settings.

Anyway, all of that is hard to do in the current setup - each subsetting would need its own Events instance at present. I'm thinking about using @preact/signals-core instead, but it has the problem that signals always have an initial value, and Obsidian plugin settings don't have a value until they're loaded from disk... which can't happen till plugin load, which screws up the necessary initialization order of services. (I really wish Obsidian would just load the plugin data automatically before instantiating the plugin instance... it'd save so many headaches.)

So, yeah, the biggest holdup to finalizing the settings API is having a clean way of creating monitorable value streams without needing to write my own rxjs replacement, too... though my local copy of ophidian/core has a draft or two of such. :wink: The new html module I'm working on in core right now is a redom-inspired Obsidian-specific HTML framework for use w/JSX, and I want to support binding it to some kind of value streams as well.

Unfortunately most tiny FRP libs seem to lack critical features somewhere:

And the big ones are, well, big. As in bigger than many Obsidian plugins' entire built main.js files. most.js's minified code is larger than the source of my drafts for a tiny FRP, including lots of doc comments. :grin: (And rxjs is an order of magnitude larger, bigger than most Obsidian plugins.)

So if you happen to know of a good tiny FRP lib with decent performance and typings, that can do lazy, on-demand subscriptions and value streams with no initial value, packaged as ES modules and implemented as operator functions so it can be tree-shaken down to what a given plugin needs... please do share. As much fun as I have writing draft FRP libs, getting them production-ready with docs and tests and such is more of a chore. :wink:

If I don't find one, my likely next approach is to just bail on delayed-start as part of the FRP framework itself, and use Promise<Signal<T>> as SettingService's main event source, and augmenting Preact signals with the other bits Ophidian needs for general-purpose FRP and HTML rendering. This won't likely change useSettings(), though, except that setting mutators will have to return new objects instead of mutating in place. But some direct access stuff for SettingsService might change, and I might need to make it so that the PluginSettingTab doesn't get registered with Obsidian until after the SettingService finishes the data load.

sytone commented 1 year ago

I have not worked with a FRP library so cannot help there. Was looking at the event library to handle notifications through the systems for call backs at one point.

sytone commented 1 year ago

This is where I know I am going the wrong way. I wanted to move the rendering logic to a separate service and then have the QueryRenderChild added as child of the MarkdownPostProcessorContext. That class needs to extend MarkdownRenderChild.

I tried to get the use functions to load the object in QueryRenderChild nd not pass them through but kept on getting no context errors. Assing it directly works but looks like it is not cleaning it self up when pages load so need to look at how all the unloads work.

MarkdownRenderChild

pjeby commented 1 year ago

Yeah, one thing there is that you don't want Service for things that aren't singletons you want to have hang around for the life of the plugin being loaded. That's what inheriting from Service or using use.service() does -- make it load() and unload() with the Bootloader, which in turn is tied to the Plugin.

Second, you don't have to fork context to create things if you just want them to have a context. You can just pass the context as a constructor argument, or send in a Useful thing. That is, like this:

class AThing {
    use: Context
    constructor(ctx: Useful = use.this) {
        this.use = ctx.use;
    }
}

Now AThing will discover its own context when created by use(), but you can also create it manually by passing it any other object with a use property. (e.g. new AThing(this) if this is a service or plugin or another Useful thing - i.e. a thing with a use: Context property.

You should use this approach for your render child stuff: just keep your existing Component subclasses but add a constructor argument you can pass the context into. No need to fork or use services for components like this -- the examples in to-use of forking for requests are intended for a web server architecture where lots of different components might need to be request-specific and live and die by that request. Per-request singletons, as it were.

But Obsidian doesn't have anything like that, just stuff like renderChild where it's not really a new context - just a thing that lives in the existing context and doesn't have any new, context-specific singletons that need to be shared among a bunch of things.

Another tip -- if you want to simplify stuff like access to Workspace, you can just do stuff like this in your main.ts:

use.def(Workspace, () =>use(Plugin).app.workspace);

And then you can just this.use(Workspace).on(...) and the like. Or set ws = this.use(Workspace) as an instance property or whatever.

pjeby commented 1 year ago

Come to think of it you can use use.def(Workspace, () =>use(App).workspace); too, since Ophidian now supplies a use.def for App as well. (Honestly I'm thinking of adding an app export to Ophidian so that you can just import app anywhere you need it.)

sytone commented 1 year ago

ok, I just played around with something similar where I passed the service into the render child that extends MarkdownRenderChild. From that I called use off that service and got the instances of the classes I needed.

I tried to just use 'use' off the class via the base functionality of to-use but it kept complaining there was no context, which I know there was as the parent class definitely had it and was able to find the services.

At that point I could have just passed the objects over be at least this way the parent does not keep on having to add more object to be sent. It just sends itself and the child render class pulls what it needs out. Updated the PR with the newer approach. This also fixed the unload issue where the rendering was not being destroyed causing a continual increase in executions when a page was modified.

pjeby commented 1 year ago

Yeah, I thought I was clear on the distinction in the to-use docs, but maybe I wasn't:

That's what the Useful interface is for: the idea is that you can make a generic class or function that accepts any Useful object to get context from, without needing to know anything else about that object.

There's really no such thing as a "parent" class in to-use: just a context you can get configuration and singletons from. If you have more than one of a thing and they're not scoped to some sort of execution context (like a web request), it's not something you'd look up via use().

Take for example PerWindowComponent in Ophidian. PerWindowComponent is not a singleton or a service - it takes a context object as its first constructor parameter, and is created by a WindowManager service. Code that needs to get a per window component ask the WindowManager, which then makes one or returns an existing one. This is a handy approach when you have more than one of a thing, but they are still well-known in some way (i.e. as the instance of a thing "for" a particular window or whatever).

I wouldn't take that approach with a renderChild, though, because they're disposable one-offs, not well-known instances. I jsut bring it up to show you there are many ways to manage things between "one-off with a context" and "total singleton".

If you have any suggestions on how to clear this up in the to-use docs, I would appreciate it. I recall briefly referencing some of this stuff in it, but I was mostly focused on singletons providing services rather than classes that consume services but don't provide them.

sytone commented 1 year ago

ok, so one the plugin is set in the global container via use = use.plugin(this); calls to this.use will be using the global context/plugin context and all services are singletons in this container, correct?

As an example at this point I want to setup a LoggingService to return a logger customized for the class/module via a name. So in the plugin or any other class that inherits from Service I can call logger = this.use(LoggingService).getLogger('modulename');. This will return a logger class pre configured by the LoggingService, the LoggingService itself is a singleton in the context for the Plugin. The returned Logger class will be independent in this case and it does not need any context as the parent class (LoggingService) which is the singleton handles it all.

For the rendering approach, in plugin I set a private property to the QueryRendererV2Service by using this.queryRendererService = this.use(QueryRendererV2Service); This would mean it is now in the context for the plugin and avaliable for services. In this case it is not really used by other classes but is a component that sets itself up in the onload function which is called via Bootstrapper looking at the library code.

During the load it is registering a Markdown Code Block Processor which should be called when Obsidian hits a codeblock when rendering preview/read mode. When it is called via Obsidian it parses the codeblock to get the config out and then adds a child (QueryRenderChildV2) to the MarkdownPostProcessorContext provided by Obsidian, which is a Component. This child is new every time as the configuration and rendering is seperate per code block.

The object I have added takes the HTML element to update, configuration, MarkdownPostProcessorContext and then this, which is the QueryRendererV2Service which is in the Plugin's context. In the QueryRenderChildV2 class I am using the passed in service and calling use of it as it has the global context still, it allows it to grab things like the plugin, logger, as well as the render and query adapters from the global context.

So reading through what you have said instead of the service being passed, I can pass the context that QueryRendererV2Service has to the child objects? So instead of passing the service over via this I should pass this.use which should be the Context object and from that I can get the singletons already registered in the plugins context.

Sorry for the ramble trying to get it out of my head to make more sense.

pjeby commented 1 year ago

ok, so one the plugin is set in the global container via use = use.plugin(this); calls to this.use will be using the global context/plugin context and all services are singletons in this container, correct?

Yep. In to-use terms, it's just the plugin context, as the global context can't be used except to define defaults. But from the point of view of everything in the plugin, it's definitely "global" in that sense. In services.ts, it's also captured as rootCtx, and you can get access to it by calling getContext() with an object that might have a .use property. If the passed in object doesn't have a use property and the plugin has been initialized, it returns the root (plugin) context.

Sorry this stuff isn't documented, but it's very much in motion -- I think I only added getContext() a month ago.

The returned Logger class will be independent in this case and it does not need any context as the parent class (LoggingService) which is the singleton handles it all.

Yeah, if an object doesn't use any other singletons it doesn't need a context.

For the rendering approach, in plugin I set a private property to the QueryRendererV2Service by using this.queryRendererService = this.use(QueryRendererV2Service); This would mean it is now in the context for the plugin and avaliable for services. In this case it is not really used by other classes

Yep. You can also just call this.use(Whatever) in the plugin onload() without keeping a reference to it, if the service just does its own thing and you don't need to talk to it from the plugin.

And you may not even need to do that much, see below...

So reading through what you have said instead of the service being passed, I can pass the context that QueryRendererV2Service has to the child objects? So instead of passing the service over via this I should pass this.use which should be the Context object and from that I can get the singletons already registered in the plugins context.

Yes. Exactly. This lets you make things more loosely-coupled, as the parent doesn't have to "spoon feed" the child everything it needs. In fact, you can write things the other way around, by making the child depend on the renderer service, instead of needing the service to create the child.

That is, you could just register a postprocessor that creates an instance of QueryRenderChildV2 and gives it a context, and then lets it look up the render service from there. At which point the plugin would not need to create the rendering service at all!

That's the true freedom of dependency injection - it lets you work "bottom up" instead of "top down". By default Obsidian architecture is sort of paternalistic, you have parents needing to know all their children's business and manage their lives. In the Ophidian model, you just let the kids do their own thing, and they have a context they can call on for whatever shared resources they need -- without the parents needing to know or care about the details.

In the limit case, an Ophidian based plugin need not do anything except kickstart any services that represent user-visible features, and the services shouldn't need to reference the plugin except to access Obsidian plugin-level APIs (like view, postprocessor and command registration), and thus only use the raw Plugin interface without needing to import the plugin class from main.

The ideal really is that the plugin can import services or utility classes and depend on what features they have, but the services and utility classes should never need to depend on the plugin except as an access point for those plugin APIs (or as a place to getContext() from, I suppose).

pjeby commented 1 year ago

This commit actually showcases a bit how some of the above is supposed to work, actually:

https://github.com/ophidian-lib/core/commit/03dd418c9c1761fb5392c62c052dc74fb3d750d1#diff-5536dabb3779843c6bc44bccbbbf02dc58b5c7fb4071b74f933a52119f52ce3eR46

If you look at how useSettings is implemented, it takes an owner argument, but doesn't do anything with it except use it to register an event and get the context (and it falls back to the root context if the owner doesn't have one). It then does stuff with the SettingsManager behind the scenes. This is an example of how you can create nice APIs that don't require explicit service management. If you useSettings() the settings service comes along for the ride, but you don't have to create it, put it in a property, remember what property you put it on, etc. etc. All you care about is that the return value of useSettings() has onChange() and update() methods.

In the same way, you could define a function like, "createQueryRenderChild()" or something, that takes an owner plus any needed parameters, and do the same thing without needing the consuming component to know anything about the underlying details. Or you can just include an owner as a class constructor argument. Or heck, you can just call getContext(null) and get the plugin context as long as the plugin constructor has already run.

When I was first writing Ophidian I thought it might be useful to have child use() contexts for some things... but the more experience I get the more I see those are extreme specialty cases. Like for example, I have considered doing some note indexing stuff where a note would get a context to look up computed field data. For any version of a note, there is always one answer to a question like, "what tags does this have" or "does this link to note X". So a context where you're looking up functions for computing those things makes sense and is a valid reason to fork contexts.

But that's an extreme special case compared to normal plugin development.

Another possibility is that if you have something like a remote login connection to a database or something, and you don't want to cache things across connections, you want clean setup each time, so you want to have "per-connection" singletons, then that's another place where forking makes sense - any components that use() the connection would then be scoped there. The tricky bit is that they couldn't be Services unless the connection had its own Bootloader so as to shut down when the connection unloaded. But you could do something like that, certainly.

Anyway, these two examples are far less common than I thought forking would be when I did the initial design and wrote to-use. That's why I've recently loosened up and added getContext(), as it makes life a lot easier in the extremely common case where there will only ever be one context.

sytone commented 1 year ago

ok, I managed to wrap my head around the settings and got a second settings tab up and running for some parts. Will look at making a custom SettingsTabBuilder that will use the JSON based configuration to reduce the noise creating all the text, toggles, buttons etc. Will let you know when I have it done.

sytone commented 1 year ago

This commit has the initial implementation. While doing it I noticed a bit of duplication around the value management as the class has the running value and then you have defaults. This may be ok but just wanted to see if I missed something obvious.

https://github.com/sytone/obsidian-queryallthethings/pull/4/commits/fa4447d4eb679d626d6b67a6c585b0850f6471e7

pjeby commented 1 year ago

I'm a bit confused about the rewritten builder. What does it do differently than the Ophidian one?

With regard to the value duplication, the intended use is that you use the applySettings callback (arg 3 of useSettings()) to do any settings-dependent setup, instead of using defaults first and then changing them later, or waiting for the data load in onload. In practice, useSettingsTab is broken because it should 1) wait for the settings to load before registering the settings tab, and 2) pass the settings values along to the showSettings/hideSettings methods. I think this would streamline things, as there would only be one place to apply settings (the onChange callback) and one place to change them (in the UI created by showSettings).

What do you think?

(Bear in mind, btw, that at this point I only have one (draft) plugin doing anything with settings at all so you are working with literally the least stable part of Ophidian that is stable enough to be checked into git at all. :wink:)

Anyway, based on your feedback so far, my thoughts are that I should change Ophidian's SettingsTabBuilder to not register the tab until settings are available, and expose the settings via an auto-updated data property so show/hide settings can access it

I don't think this would break anything, but it should allow a single source of truth and updates for each settings consumer, as they will be guaranteed not to receive data callbacks (or show/hide settings callbacks) until the data is available.

pjeby commented 1 year ago

Ok, I've fixed some things with settings. The settings service now has a .current property that you can read while creating settings UI. It's not guaranteed to be non-null, though, so you still need the applySettings callback in useSettings to actually apply the settings (as opposed to displaying them in showSettings).

It's also not using events under the hood any more, and you can now return a replacement value from an update() callback instead of modifying in-place.

sytone commented 1 year ago

I'm a bit confused about the rewritten builder. What does it do differently than the Ophidian one?

I had not checked in. It now has helpers so my settings are more consistent and I do not have to repeat the setup and classes. Probably a better way to do it but playing around with this is helping me learn how it works better. I do not have a good strong skill base in Typescript/JavaScript and the ecosystem.

With regard to the value duplication, the intended use is that you use the applySettings callback (arg 3 of useSettings()) to do any settings-dependent setup, instead of using defaults first and then changing them later, or waiting for the data load in onload. In practice, useSettingsTab is broken because it should 1) wait for the settings to load before registering the settings tab, and 2) pass the settings values along to the showSettings/hideSettings methods. I think this would streamline things, as there would only be one place to apply settings (the onChange callback) and one place to change them (in the UI created by showSettings).

That makes sense, in the case of useSettings in the plugin class onload would have fired so I would still need to have the defaults in place? For the rest would useSettings execute before the onload in the Service/Component. Not sure how to easily visualize the call tree in TS.

(Bear in mind, btw, that at this point I only have one (draft) plugin doing anything with settings at all so you are working with literally the least stable part of Ophidian that is stable enough to be checked into git at all. 😉)

Something about a sense of adventure? I hope I am helping and not hindering, I am finding the conversation to be pretty insightful on my side. So thanks for that.

I will look at the changes in the next few days to see how it looks but from what you have said it makes sense. It may also make it possible for me to enable the saving of the collapse data for folding the headings in settings as a part of the settings tab builder and not have to worry about it as part of a setting for the plugin/service/component

sytone commented 1 year ago

I just tried to use current in showSettings and it was not picking up the value that is set in the data.json. The settings tab is showing fine with the placeholder value but using current or the property off the class that has showSettings is not returning anything. Looking at the apply settings callback I can see the value is being read and is in the settings object.

May be a issue with timing? Checking in the change, the 'AlaSqlQuery' class was the one I was playing with to test this out.

sytone commented 1 year ago

Nope, I was a idiot and did not fix a variable reference

pjeby commented 1 year ago

That makes sense, in the case of useSettings in the plugin class onload would have fired so I would still need to have the defaults in place?

You can just have the applySettings call do whatever you would've done with those settings in onload, though. It's a bit like running an onLayoutReady callback to do stuff when it's safe.

I've just refactored the settings service and useSettings to make this easier for one-time setup, though: the service now has each() and once() methods (and corresponding callback arguments in useSettings()). So you can do one-time setup (like registering commands that depend on the settings) in a once callback, and apply active settings (like adding or removing classes to the dom, adding/removing event handlers, etc. that need to be done whenever the settings change) in an each callback.

each was previously the onChange() method and the applySettings callback - I've renamed these for consistency and simplicity. (Also hopefully clarity as to the fact it's called with each value the settings have, starting with the initial loaded settings.)

I think this was the right thing to do because the settings tab was already hacking its own version of once to register the settings tab!

So, basically, if you need settings to be valid so that callbacks can happen (e.g. commands, postprocessors, etc.), set them up in the once callback, as that guarantees settings are loaded. (Services should also follow this policy if they use settings.) If you need to know when settings change because things in the environment need to be turned on or off or reconfigured, use the each callback to update them.

So for example, if I was doing something like the original sliding panes plugin, and had a settings toggle for it being active, the each() callback would add or remove CSS classes (or add/remove event handlers) depending on the settings it receives each time. At startup it gets the value (with defaults if applicable) from the settings load. After that it gets it every time the value is toggled, whether it's by a command, button, settings dialog, or whatever. (And of course settings are saved based on that as well.)

Anyway, the overall setup is intended to allow one to avoid the misleading nature of async onload, which superficially appears to work but internally screws up initialization order (because child components can finish their onload() even before the parent does, if they are sync and the parent is async). For monolithic plugins this isn't a big deal, but if you're assembling a plugin out of generic services and components, being able to rely on what order things happen in is a bit more important.

sytone commented 1 year ago

ok, that is much clearer to work with, I had added booleans to make sure only some actions would run one on start-up. I can now remove this and use the once call back.

An example for what I am doing is when the plugin starts it can run a set of SQL based queries to make/change in memory tables for later use if needed. This should only run once on startup/enable of the plugin. I added the boolean check so when the settings were updated in the settings table it would not just run again and break any existing data.

I can now move that to once so it will only execute once and then the users can disable/enable the plugin to force it or restart obsidian which makes it much simpler.

Ill update shortly and push an update to see how it rolls.

pjeby commented 1 year ago

What's coming next is the ability to use signals and effects: Ophidian will re-export the preact/signals package, allowing you to manage settings like this:

import {calc, effect, useSettings} from "@ophidian/core"

class MyServiceOrPlugin  extends Whichever {

    settings = useSettings(this, {someSetting: something as SomeType});
    @calc get someSetting() { return this.settings.current?.someSetting; }

    onload() {
        this.register(effect(() => this.doSetupForSomeSetting()))
    }

    doSetupForSomeSetting() {
        if (this.someSetting == Option1) {
            // code to set up for option 1...
            return () => {
                // code to UNDO setup for option 1...
            }
        }
        if (this.someSetting == Option2) {
            // code to set up for option 2...
            return () => {
                // code to UNDO setup for option 2...
            }
        }
    }
}

Basically, anything you register as an effect is a callback that can use computed values (e.g. via calc decorator) and will automatically get called again if they change. settings.current is already effectively a calc value so anything that depends on it will be re-run when the settings change. Anyway, "effects" can return "undo" or "cleanup' functions, and they will be called if any (monitored) data changed that the effect used during the last call to it. (And also when the plugin exits, via the register part shown.)

This is going to make it super easy to have plugin settings with complex effects, without needing to restart in order to change them, as long as it is something that's actually reversible.

There are two small catches: First, it only detects !== changes to values that are either computed (using @calc or compute) and derive from a signal(). SettingsService, if you look at the code, is using a signal ("version") and a compute value ("cloned") under the hood to implement the .current property, so any @calc or compute or effect that accesses .current will be re-run when the underlying .version of the settings change. So anything you want to monitor and update based on has to be defined in some way using the signals package.

The second catch is that effect functions should do as little calculation as possible in the body of them, i.e. anything that can be moved to a @calc/compute, the better. That way, you won't be undoing and redoing stuff every time any setting changes. That's why the @calc is in the example above: it isolates the effect so it will only re-run if that setting changes (or any other settings it uses.

Apart from these two things to watch for, it's a huge benefit for complex, dynamic features in plugins. It's also hugely useful for rendering UI, like for example having an effect that adds an is-selected class to the current (computed or assigned) selection position, and returns a callback to remove it. Instead of having to centralize that code with anything that updates the position, you just write an effect that basically says, "make sure the current item has this class and takes it off when it's not current". Bam!

The thing I'm currently working on for Ophidian is a special kind of effect that will let you write things that act like a cross between an effect and an async function, where yield pauses the function until something changes (that the function read since the last yield) or until a promise settles. (And you can yield undo functions.) My Calibre plugin has some complex async remote data loading logic I want to replace with a task function that does something like:

while(true) {
    if (queryToString(currentlyLoaded) !== queryToString(currentSettings)) {
         const newResults = yield *waitFor(requestURL(....));
         // could have changed while we waited
         if (queryToString(justLoaded) === queryToString(currentSettings)) {
             // set the new results, triggering UI updates and changing `currentlyLoaded`
         }
    } else if (currentPosition > currentResults.length - 25 && canLoadMore()) {
         // load more results and extend the current result list, similarly checking results 
         // are still valid after yield
    } else yield;   // nothing to do, so wait for something to change and trigger us again
}

Wrapping that in a "special effect" of the kind I'm writing will simplify a lot of logical flags I'm currently using to simulate all that. Writing it this way automatically makes it "single-threaded" in a sense, automatically rate-limiting search calls to the response speed of the server, and doing infinite scroll loading... but only if the user isn't in the middle of typing stuff in the search box.

Granted, this style has its own gotchas: you need to be careful not to double-yield, i.e. have two yields without checking any values in between, and the first draft of what I wrote above had them. The if-else and yield only after all conditions fail was necessary to avoid it. I think I need to think some more about how to write these effectively, so that there's a standard pattern or rules you can follow to get a "safe" task. But roughly speaking, they need to be arranged as an event loop of this kind: checking for various conditions and potentially yielding in the middle to accomplish things, but eventually dropping back to check conditions again, and only yielding at the top level of the loop when no conditions match.

An easier-to-read version might be:

while(true) {
    if (queryToString(currentlyLoaded) !== queryToString(currentSettings)) {
         yield *this.loadNewResults();
    } else if (currentPosition > currentResults.length - 25 && canLoadMore()) {
         yield *this.loadMoreResults();
    } else yield;   // nothing to do, so wait for something to change and trigger us again
}

yield * lets you call other generator functions, which can then do their own nested event loops or wait for promises or whatever. Doing it this way makes it easier to verify the correctness of the event loop structure as not double-yielding -- there is clearly only one yield per loop here.

Anyway, that about summarizes coming attractions, I guess. 😀 I've just pushed the @calc and @prop decorators too, if you want to play with them. (Your tsconfig.json needs "experimentalDecorators": true and "useDefineForClassFields": false in order for them to be compiled correctly, though.) If you want more background on the underlying signals concept/library, here's the API Guide.

sytone commented 1 year ago

I'm going to have to read that one a few times 😀

sytone commented 1 year ago

I'll look at signals to see how that maps to other callback models I know in my head and see if I can impliment in the next few weeks. Still cleaning up my plugin and refactoring items around. Also trying to get jest to work but the whole js version ecosystem of modules and requires is fighting against me.

pjeby commented 1 year ago

You don't need to worry about signals if you're just using the once/each/useSettings API. But, if you want to know, signals are basically spreadsheet cells - values are input cells, and computes/effects are cells with formulas. Like a spreadsheet, when you change an input cell, the formulas get recalculated.

In the last few weeks I've realized that preact/signals isn't actually going to work for what I want to do, so it will ultimately get replaced; the API I'm going to end up with will use someSignal() and someSignal.set() instead of someSignal.value. But, if you're just using the current settings API, you don't need to worry about that, because the once/each/useSettings APIs aren't really changing. The only thing that might be changing is from .current to .current() to get the current settings.

There is one other thing I want to change, though -- I want to drop hideSettings() as a thing and just have showSettings() take a Component you can use to register() things to unload in place of hideSettings(). I've been meaning to take a look at your code to see if that'll break anything but haven't got around to it yet. (Specifically, changing it so showSettings() is called with a Component as the first argument instead of the Builder.)

pjeby commented 1 year ago

Ok, the latest commit drops hideSettings() and passes a Component to showSettings() instead, so you can .register() callbacks on it if you need to do stuff on closing of the tab.

There's also now a narrower set of exports for the signals API, and it's a bit different from the one provided by @preact/signals-core. (At the moment it's just a thin wrapper over their implementation but it will eventually be replaced with an in-house version.)

That will hopefully be it for backwards-incompatible-ish changes to Ophidian for a little while at least -- I hope :wink:. (It probably will be, just because I will be mainly writing new stuff in the near future, like adding an indexing service base class to handle a lot of the boilerplate for managing live indexes in Obsidian.)

Anyway, please let me know if you have any further questions or feedback, or would like me to look something over.

aidenlx commented 11 months ago
    onload() {
        this.register(effect, () => this.doSetupForSomeSetting())
    }

Just noticed a small oversight in the example code. Here's the corrected version:

    onload() {
        this.register(effect(() => this.doSetupForSomeSetting()))
    }
pjeby commented 11 months ago

That's weird... I wonder how I could've typoed it that badly. Good catch; I've updated the example.

sytone commented 11 months ago

That will hopefully be it for backwards-incompatible-ish changes to Ophidian for a little while at least -- I hope 😉. (It probably will be, just because I will be mainly writing new stuff in the near future, like adding an indexing service base class to handle a lot of the boilerplate for managing live indexes in Obsidian.)

What are you covering in the indexes? Interested as my plugin is all about querying data in obsidian and allowing it to be displayed/exported as you want.

pjeby commented 11 months ago

The current draft indexing code is here: https://github.com/ophidian-lib/core/blob/master/src/indexing.ts

You subclass either AbstractIndexer or NoteMetaIndexer and add a parse() method, and the result is a Service.

Indexers have two type parameters: the type of "item" being indexed (e.g. TFile in the case of NoteMetaIndexer) and a map type of keys to value types. For each item, the index can track an unlimited number of key-value pairs, where keys must be strings, symbols, or numbers, and values can in principle be anything. (But in practice they'll usually need to be strings, symbols, or numbers, since objects would be compared by identity.)

So, as a simple example, you could make a NoteMetaIndexer for properties, that used property types as keys and property values as values. (With multi-valued properties being treated as one key-value pair for each property value.) You'd then be able to iterate over all notes with a particular property (key), or that have a specific key-value pair present. (Or get counts of the same, or retrieve the first item with a specific key-value pair.)

You're not limited to using note metadata, you just have to have a synchronous parse() method that emits key-value pairs for some input item (which doesn't have to be a note or even a file). If your index needs to parse markdown then you just need to set up logic to handle calling the index add() method after the data is (asynchronously) loaded.

Note, though, that if you're creating an index with an item type that isn't TFile, you need to understand a bit more about how the indexing works to use it properly. Items are implicitly assumed to be mutable objects in the sense that they have a long-lasting identity but with changing keys and values. So if you use an immutable value as an item type (e.g. file names), then a file being renamed means you would have to delete() the old name and add() the new one. And if you're trying to do something like index individual tasks or some other in-document items parsed from markdown, you would need to save the objects you created on each parse so that you could delete() the old ones and add() the new ones.

None of these issues apply if you're just indexing TFiles as the "item", though, since TFile objects always represent the same file, even if they've been moved or renamed or deleted. So the very simple approach shown in NoteMetaIndex should work fine for indexing files (as opposed to items generated from their contents).

Currently the only place I'm using the indexing code is in a draft version of my Calibre Connect plugin, to index which notes are linked to specific books in Calibre via a Calibre-URL or Calibre-ID property. So it uses a NoteMetaIndexer with key value pairs representing Calibre library codes and book ID numbers extracted from the properties by parse(). It then adds plugin-specific methods to look up, open, or create "book notes", using the inherited index methods.

Anyway, indexing is currently for unordered values only. If you want to do something like iterating over files in last-modified order or something like that, you'd currently need to iterate over the values for the key and then sort them or put them in some other kind of index. I may eventually add the ability to have ordered keys, and/or add more classes to make async indexing or parsed-item indexing easier, but I don't have a need for either one myself at the moment.

pjeby commented 4 months ago

Just a heads-up for anyone following this... Ophidian/core is going to have a bit of minor upheaval soon. I've just released the first version of a new library (Uneventful), which will be used to upgrade certain internal parts of ophidian/core.

Specifically, the contents of internal modules cleanups, signify, and eventful will be mostly gutted and replaced with their better equivalents from Uneventful.

If you're not using anything that comes from inside those submodules you probably won't be affected. I do intend to try to leave a fair amount of (deprecated) wrappers in place even there, to minimize disruption and ease migration, though. (Especially since some of my plugins use them currently.) And I'll be aiming to not disrupt anything else either (preferably at all), though there may be some changes in how the newer Settings UI stuff ends up working.

At this point, I've bumped the npm release of core to 0.0.22, to have a stable version number you can stick with for things as they are now, and not get surprised by anything that gets pushed to master in the future.

So, tl;dr: pin to 0.0.22 for now if you don't want to mess with this, there will be some later 0.0.x releases with deprecated wrappers that keep (hopefully almost entirely) the same outward API, and then some 0.1.x releases that will drop all the deprecated stuff and just use the newer APIs from Uneventful. There will also be a ton of new functionality coming down the road based on Uneventful, that will make interacting with certain Obsidian APIs a lot easier, much of it in the area of settings and settings UI.

No timelines on any of this yet -- there are a few more things I want to do with Uneventful before I start any of this work, and it may be a while before I start checking any new stuff in to core, so hopefully that gives y'all plenty of time to switch from the github version to the npm version, or at least make a note of what git version you're currently using. :wink:

Let me know if you have any questions or concerns.