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

[feature request] BehaviorSubject to Signals migration #504

Open
ShacharHarshuv opened this issue Sep 18, 2024 · 6 comments
Open

[feature request] BehaviorSubject to Signals migration #504

ShacharHarshuv opened this issue Sep 18, 2024 · 6 comments
Labels
enhancement New feature or request

Comments

@ShacharHarshuv
Copy link

ShacharHarshuv commented Sep 18, 2024

In the spirit of writing migration scripts to modernize Angular code, I want to propose the following script. Let me know if you think this is something that could fit the library.

The assumption here that everything that a BehaviorSubject is used for, a signal can be used instead, and I think it is possible to write a (relatively) robust script to do that migration. The script will:

  • Replace member instantiation of new BehaviorSubject with signal, while keeping any used generics / inferred types.
  • Replace .next with .set
  • Rename the member if it ends with $
  • Replace myValue$.value usages to myValue()
  • Replace any other myValue$ with `toObservable(myValue)
    • caveat: it might not be in an injection context. A possible solution of it will be duplicating the member in instantiation so that we'll have both myValue as a signal and myValue$ as a stream. In that case, if there is no $ as a postfix, we'll have to add it.
@jdegand
Copy link
Contributor

jdegand commented Sep 19, 2024

And convert .next to .set?

@ShacharHarshuv
Copy link
Author

Yes, sorry. Added!

@eneajaho
Copy link
Collaborator

Hi, something is coming 🙌

Let's wait a little bit more 😬

@eneajaho eneajaho added the enhancement New feature or request label Sep 19, 2024
@ShacharHarshuv
Copy link
Author

ShacharHarshuv commented Sep 19, 2024

Do you mean you are working on it, that Angular's core team is working on it, or that you think it's premature to add such a feature now?

@eneajaho
Copy link
Collaborator

I've been working on a prototype for myself to do these kinds of migration, but nothing ready yet.

@ShacharHarshuv
Copy link
Author

ShacharHarshuv commented Sep 19, 2024

Cool! I also thought that a next step could be converting something like that:

combineLatest({
  a: toObservable(this.a),
  b: toObservable(this.b),
})
.pipe(
  map(({a, b}) => this.a() + this.b()),
  distinctUntilChanged(), // ... and other operators that don't matter with signals like share etc
);

to

computed(() => this.a() + this.b());

Might be harder than I think but it feels like it could be possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants