chore: make engine schema v3 ready (part 1)

[jonathanyeung]

chore: make engine schema v3 ready (part 1)

Part 1 of making schemas engine v3 ready by removing references to the engine.schemas property.

A few notes:

• Moved SchemaUtils from out of dnode.ts into its own file

Notable change of behavior with childOfDomainNamespace

This change will alter the behavior of childOfDomainNamespace. Right now the logic of this setting is to see if the domain has an existing schema set to namespace. If it does, then two hierarchy levels will be used foo.bar, whereas if no schema exists for foo that is declared a namespace, then only foo is used.

This PR has altered the behavior to just use 2 levels if up to 2 levels exist, otherwise it'll use 1. The reason for this change of behavior is because v3 engine necessitates schema lookups (and note lookups) to be async. This would then cause genNoteName / genNotePrefix to need to be async, which causes a cascade effect of functions needing to be marked async - some of which are quite difficult to alter, such as the note traits API surface.

I don't think anyone will notice this behavior (it might even be more desirable?), but @kevinslin and others, let me know what you think.

[kevinslin]

looks like tests are failing

[kevinslin]

I don't think anyone will notice this behavior (it might even be more desirable?), but @kevinslin and others, let me know what you think.

Trying to wrap my head around this. Can you list out an example for this?

[jonathanyeung]

I don't think anyone will notice this behavior (it might even be more desirable?), but @kevinslin and others, let me know what you think.

Trying to wrap my head around this. Can you list out an example for this?

Given the following Notes:

.
└── science
    ├── biology
    │   └── bioengineering
    └── physics

New Behavior: If you have the bioengineering note open, then your scratch note will be science.biology.scratch.2022.... If you have science open, it'll be science.scratch.2022.... This behavior holds no matter what schemas you have configured. You get 2 levels of hierarchy in the auto-gen'd note name if your current note is at least 2 layers deep, otherwise you get 1 level.

Existing Behavior: If you have the bioengineering note open, then your scratch note will be science.biology.scratch.2022... if you also have a schema for science which is set to namespace:true. If you don't have the schema created, then it'll be science.scratch.2022...

At least, this is my understanding by the docs and code inspection.

[jonathanyeung]

looks like tests are failing

Circular Dependencies are back to haunt us again. The failures in CLI are caused by circ dependencies in common-all. I tried some basic rejigging, but they're failing persistently. Looks like we need to get rid of those before we can merge this, sigh.

update: PR here: https://github.com/dendronhq/dendron/pull/3577. This PR is now dependent on that one.

[hikchoi]

so if I'm understanding this correctly, now if the addBehavior is childOfDomainNamespace, the top two levels are prefixed regardless of the second deepest being a namespace or not?

I think childOfDomainNamespace is limited and bit arbitrary to only allow two levels deep from the domain to begin with, but now it's a bit more confusing because it has nothing to do with the domain's children being a namespace or not. A bit of a pickle since keeping the original behavior is tricky right now, but the other option is confusing. I think for now as much as we hate it, we allow the confusion and try to resolve it in a separate PR.

Possible idea to resolve this in the future:

• deprecate childOfDomainNamespace, introduce / implement childOfNearestNamespace, which looks for the closest ancestor which is a namespace.
• This makes is scale beyond the first two levels of hierarchy, and the name makes more sense.
• problem: this still probably needs to be async because we have to walk up the hierarchy.

[jonathanyeung]

@hikchoi - you summarized it well, thanks.

I'm in favor of deprecating it, and maybe replacing it with something else if users actually want it. As it stands now, the usability of this setting is quite low... I had to do code inspection to understand what it's trying to accomplish.

[kevinslin]

got it - thanks for the explanations. are we able to just change this to async for now? it looks like this is only being called by getNoteName in [[../packages/plugin-core/src/clientUtils.ts]]

getNoteName is being called in 4 places - 3 of them are in modifyPickerValueFuncwhich requires that onTaskButtonToggled be made async but it looks like there are no conflicts to do this

1 is in createnotetrait which is called in [[../packages/plugin-core/src/commands/CreateNoteWithTraitCommand.ts]] but it looks like there shouldn't be any issues making that async either

[hikchoi]

1 is in createnotetrait which is called in [[../packages/plugin-core/src/commands/CreateNoteWithTraitCommand.ts]] but it looks like there shouldn't be any issues making that async either

I think the hesitance is that changing it to async means onWillCreate now has to be async, which is a breaking change in the API.

Not too sure about the implication of that in terms of api design, but at the very least this will break all existing custom traits.

That being said, we did set out that note traits is a germ stage feature and will have breaking changes in the future.

[jonathanyeung]

I think the hesitance is that changing it to async means onWillCreate now has to be async, which is a breaking change in the API.

Exactly - this is the difficult change. It's a breaking change for anyone who has traits, and it causes a lot of changes in our own note traits code.

I'm also still not convinced the traits API's should be changed to be async - I think it makes more sense to add more data into OnCreateContext rather than let the API's be async.

[jonathanyeung]

@kevinslin - as a follow up to our discussion last night, I've decided to leave the implementation to as it currently is in the PR. Repasting from my comment above:

Given the following Notes:

.
└── science
    ├── biology
    │   └── bioengineering
    └── physics

New Behavior: If you have the bioengineering note open, then your scratch note will be science.biology.scratch.2022.... If you have science open, it'll be science.scratch.2022.... This behavior holds no matter what schemas you have configured. You get 2 levels of hierarchy in the auto-gen'd note name if your current note is at least 2 layers deep, otherwise you get 1 level.