Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

inlang project: async function calls should be async #1290

Closed
1 of 3 tasks
samuelstroschein opened this issue Aug 25, 2023 Discussed in #1272 · 13 comments
Closed
1 of 3 tasks

inlang project: async function calls should be async #1290

samuelstroschein opened this issue Aug 25, 2023 Discussed in #1272 · 13 comments
Assignees
Labels
scope: inlang/sdk Related to source-code/sdk. type: refactor Code being refactored to improved performance or long-term maintainability.

Comments

@samuelstroschein
Copy link
Member

samuelstroschein commented Aug 25, 2023

Discussed in #1272

Originally posted by samuelstroschein August 22, 2023

Problem

  • treating async operations sync will lead to bugs in apps
  • awaiting async operations with a dedicated init() is unergonomic

The reactive system treats async operations like inlang.lint.reports as sync operations leading to the requirement to call await inlang.lint.init() before accessing lint reports. Awaiting init is arguably inconvenient and will definitely lead to "gotcha" moments. More importantly though, awaiting init will lead to bugs.

Example

The production code below struggles with awaiting an update operation and introduces a workaround by awaiting a promise that will eventually resolve. A gateway to bugs by thinking "this is sync, why is my message not immediately update?", forgetting to establish such a workaround, maybe the timeout is too fast, etc.

https://github.com/inlang/inlang/blob/3c4b975a47c427dbb8cbdf565eedca0a75cdd0da/source-code/core/app/src/wrappers/withSolidReactivity.test.ts#L259-L261

Sub-tasks

@maige-app maige-app bot added type: bug Something isn't working scope: inlang/sdk Related to source-code/sdk. labels Aug 25, 2023
@samuelstroschein samuelstroschein added type: refactor Code being refactored to improved performance or long-term maintainability. and removed type: bug Something isn't working labels Aug 25, 2023
@samuelstroschein
Copy link
Member Author

@janfjohannes do you want to implement an async primitive while you factor out lix/reactive, see

#1142 (comment)

@janfjohannes why does lix depent on inlang/app?

If we want to share reactive primitives, lets extract the reactive primitives and adapters from inlang/app to lix/reactive-primitives or something.

The logical deduction is that "inlang is a lix app". Not the other way around "lix is an inlang app"

@janfjohannes
Copy link
Contributor

@samuelstroschein i did a first version using the same solid to subscribable method as in inlang/app, there were stil too many open questions to do somthing propper and did not want to block @NilsJacobsen any more.

i think we need a proper working session to make a good plan as you said, this should play nicely with awaitng and async. i really currently fancy something using async iterables as this allows awaiting and has native js support but i am not 100% sure about this.

@samuelstroschein
Copy link
Member Author

@janfjohannes we should ask in the solid discord what they recommend/what's upcoming in solidjs 2.0. i heard that solid 2.0 has first class async support. coordinate with @ivanhofer pls. it's clear that we need sync and async reactivity for both inlang and lix/client

@janfjohannes
Copy link
Contributor

@samuelstroschein i checked in the rxjs froums and they have solved it in rxjs 7 like this, and they also have a subscribable:

// these are awaitable 
await rxjs.lastValueFrom(inlang.someapi.something)
await rxjs.firstValueFrom(inlang.someapi.something)

however as in a reactive system the subscribable never completes until the reactivity is disposed we probably need somthing liek await nextValueFrom(inlangApiObservable) wher the promise would resolve when the next value is produced after the subscription is started

@janfjohannes
Copy link
Contributor

@ivanhofer you know solid much better than me and also had interactions with the team before, could you get feedback for a best practice from solids perspective?

@samuelstroschein
Copy link
Member Author

@janfjohannes the solid community is friendly. I would ask in the #reactivity channel. Say that you come from inlang.

PS we are in elaborating a sponsorship for solidjs. If solid (2.0) suits out use case for lix and inlang reactivity best, we sponsor them for ongoing support/use inlang to build their i18n stuff

@samuelstroschein
Copy link
Member Author

It seems to me that we need an asyncSignal primitive. Solid offers resource maybe that's what ne need.

signal()
asyncSignal()
derived()
effect()

@NilsJacobsen
Copy link
Contributor

I'm gonna make some tryouts today. Will discuss the outcome with you later.

@NilsJacobsen NilsJacobsen self-assigned this Aug 29, 2023
@samuelstroschein
Copy link
Member Author

@NilsJacobsen

The API I have in mind is

const lint = asyncSignal(() => lintMessages({}))

await lint()

Questions:

  1. How to lint all messages and then only changed messages? I guess that needs to be handled in lintMessages internally

@NilsJacobsen
Copy link
Contributor

Looks good, Under the hood, we also want lint to be an async subscibable right? And then with the adapter an async signal.
I'm gonna start experimenting with lint.

@samuelstroschein
Copy link
Member Author

Looks good, Under the hood, we also want lint to be an async subscibable right? And then with the adapter an async signal.

Yes, an async subscribable entails that we need an async signal.

@NilsJacobsen
Copy link
Contributor

Okey what I have right now is that createAsyncSubscribable

export function createAsyncSubscribable<T>(
	signal: () => T,
	init: () => Promise<void>,
): AsyncSubscribable<T> {
	const asyncSignalGetter = async () => {
		await init()
		return signal()
	}
	return Object.assign(asyncSignalGetter, {
		subscribe: (callback: (value: T) => void) => {
			createEffect(() => {
				callback(signal())
			})
		},
	})
} 

leads to this usage:

await inlang.lint() //get value
inlang.lint.subscribe((m) => console.log(m)) // access callback

await inlang.lint() // TODO: get signal, does not work right now

The whole topic is getting more and more complex. Maybe let's discuss tomorrow morning how we can make it work this week.

@samuelstroschein
Copy link
Member Author

We have a hacky implementation in place. Future update will follow

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: inlang/sdk Related to source-code/sdk. type: refactor Code being refactored to improved performance or long-term maintainability.
Projects
No open projects
Development

No branches or pull requests

3 participants