getter functions will lead to multiple breaking changes in the future

[samuelstroschein]

Problem

The Subscribable approach of the project and @inlang/lix API has a high risk of breaking changes.

We settled on getter functions because most signal implementations use getter functions. However, determining whether a property is or will be reactive in the future yields a high risk of making a wrong choice.

// reactive getter function
project.errors()

Example

1678 will add a project.name property. Shall project.name be reactive?

Should users have the possibility to rename a project in a UI like the Fink editor without reloading the entire project? Arguably yes. Is that use case obvious at the moment? No. So what to do now? Make the property reactive or not?

If we decide to make project.name static and reactive in the future, we have a breaking change. If we make project.name reactive now but it will remain static in the future, developers expect project.name to be reactive while it is not.

[samuelstroschein]

Proposal 1 - Change subscribable to an object getter

That's how Preact signals work https://preactjs.com/guide/v10/signals/. We can expose the .value property in the project API.

- project.errors()
+ project.errors

Pros

• better plain JS usage (CLI, Paraglide) with typescript working etc
• the API doesn't change whether or not an API is reactive or not

Cons

• requires effect function for .subscribe, which we will need to explain to devs

- project.query.messages.get().subscribe(( value ) => console.log(value))
+ effect(() =>
+  console.log(project.query.messages.get()) 
+ )

[samuelstroschein]

@martin-lysk @janfjohannes curious about your input regarding the proposal above. another proposal is also welcome.

a bummer that JS reactivity is not standardized. @martin-lysk please do not open a discussion if reactivity is required or not :D we made the experience that building the @inlang/editor became so much easier with reactivity that it is/was well worth it.

@NiklasBuchfink @NilsJacobsen if you disagree with the reactivity made building the Fink editor easier, please let us know

[NilsJacobsen]

For fink it was great 👍

[NiklasBuchfink]

Lol, I heard this exact discussion this morning :D https://open.spotify.com/episode/0WOjZzBZ3yDsDIuEvcbCAy?si=50e4c6641399441a Timestamp: 24:06

Agree, for SolidJS in fink it was great 👍

[janfjohannes]

@samuelstroschein i dont understand the problem description fully. but i still think we should not poolute lix with any framework specific reactivity concepts, the implementation we settled on was a compromise and not pretty but mostly motivated by having to change the editor as little as posible and i see the error object still as experimental until we know more about how to best use lix and reactivity.

i would say that the standard js ways to get a stream of values would be (from older to more modern, leaving out observables):

repo.on('error', callback) repo.errors.subscribe(error => {....}) or for await (const error of repo.errors) {...} < my current favorite as this also is in the js standard and supported by all browsers

to adapt this to whatever framework reactivity system needs should be not part of lix or maybe some extra exports in a seperate module.

[samuelstroschein]

I will update the issue on the problem later today.

The issue is unrelated to the implementation. The API we expose of Subscribable is of high risk for breaking changes.

[samuelstroschein]

I will update the issue after I implemented the directory proposal.

[samuelstroschein]

reply to @janfjohannes https://github.com/opral/monorepo/issues/1680#issuecomment-1821478152

but i still think we should not poolute lix with any framework specific reactivity concepts [...] to adapt this to whatever framework reactivity system needs should be not part of lix or maybe some extra exports in a seperate module.

No worries, we are not going to leak framework-specific reactivity.

The fact is that we need a reactivity/observable solution in the SDK. The inlang SDK and lix need to "broadcast" events to consumers [which triggers side effects like rendering the UI]. How those side effects are triggered can be a React wrapper, Solid wrapper, whatever. In any case, the SDK needs a primitive to expose those events.

for await (const error of repo.errors) {...} < my current favorite as this also is in the js standard and supported by all browsers

Async iterators seem unsuited. They are a pull-based mechanism. A UI doesn't know when to pull changes. Updating UIs requires a push-based mechanism. Observables and signals are push-based. We need "(SDK/lix to UI) Hey UI, project.errors changed" and not "(UI to SDK/lix) Hey SDK, did project.errors change?". The latter would require manual pulling all over the source code, leading to engineering complexity and a degraded user experience. The UI would not be guaranteed to react to changes in a project [because the manual pulling implementation has a naive 1s timer for example]

PS Lix has the same requirements. If an app calls repo.statusMatrix() to display changes in the UI, the call must be reactive/observable.

EDIT: signals are, interestingly enough, push and pull-based at the same time. Pull-based because no subscription is required e.g. an app can just access project.errors() with a push mechanism in the form of effect, which triggers reactive updates

EDIT 2: Edit 1 lead to https://github.com/opral/monorepo/issues/1772#issuecomment-1877842303. Thanks @janfjohannes for raising your concerns about not leaking implementation details. The comment prompted me to formulate the requirement in the SDK

[samuelstroschein]

We partially made the right design decision by settling on functions.

Good:

• static access like project.errors does not work. at any given time an async computation could occur from which errors are derived from. the parent computations must be awaited before project.errors returns a value. Hence, project.errors() must be an async function!

Bad:

• every property must be an async getter because every property depends on an async parent computation like reading the settings file