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

feat(signal-slice): add "Updated" signal for action sources #363

Merged

Conversation

joshuamorony
Copy link
Contributor

NOTE: This depends on #361 being merged, this is branched off of that feature

This is a replacement for the removal of the actionEffect API, instead it allows triggering side effects as a result of an actionSource emitting via a notifier/version signal (same general concept we used for createNotifier).

It adds a <actionSourceName>Updated signal for each action source e.g:

  state = signalSlice({
    initialState: this.initialState,
    actionSources: {
      load: (_state, $: Observable<void>) => $.pipe(
        switchMap(() => this.someService.load()),
        map(data => ({ someProperty: data })
      )
    }
  })

  effect(() => {
    // triggered when `load` emits/completes and on init
    console.log(state.loadUpdated())
  })

  effect(() => {
	if (state.loadUpdated()) {
	  // triggered ONLY when `load` emits/completes
	  // NOT on init
	}
  });

This is a much simpler API and avoids the typing issues (described in: #361)

@michael-small
Copy link
Contributor

Sounds like a fair tradeoff to me. If the pattern of this PR and the base PR is to de-couple signalSlice from formal effect blocks, then having some sort distinction between other effect in a file unrelated to the slice and effect related to the slice is a nice tradeoff.

@eneajaho
Copy link
Collaborator

eneajaho commented May 8, 2024

Hello @joshuamorony
Can you add the breaking change in the commit description if this is a breaking change?

@nartc nartc added the wip label Jul 15, 2024
@joshuamorony joshuamorony force-pushed the feat/signal-slice-signal-updates branch 2 times, most recently from f7b41fb to d21f9bf Compare July 17, 2024 10:48
@joshuamorony joshuamorony marked this pull request as ready for review July 17, 2024 10:57
@joshuamorony
Copy link
Contributor Author

@eneajaho we ended up merging #361 and this is ready for review now (no breaking changes in this PR)

@eneajaho
Copy link
Collaborator

@joshuamorony can you rebase? Then I can merge

@joshuamorony joshuamorony force-pushed the feat/signal-slice-signal-updates branch from b56852a to f96bb0c Compare July 17, 2024 11:15
@joshuamorony
Copy link
Contributor Author

@eneajaho done

@eneajaho eneajaho merged commit 1700269 into ngxtension:main Jul 17, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants