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

add TSubscriptionRef #2725

Merged
merged 10 commits into from
Oct 10, 2024
Merged

Conversation

evelant
Copy link
Contributor

@evelant evelant commented May 10, 2024

Type

  • Refactor
  • [ X] Feature
  • Bug Fix
  • Optimization
  • Documentation Update

Description

Add TSubscriptionRef for STM

Related

  • Related Issue #
  • Closes #

@evelant evelant requested a review from mikearnaldi as a code owner May 10, 2024 17:45
Copy link

changeset-bot bot commented May 10, 2024

🦋 Changeset detected

Latest commit: 55dcb28

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 32 packages
Name Type
effect Minor
@effect/cli Major
@effect/cluster-browser Major
@effect/cluster-node Major
@effect/cluster-workflow Major
@effect/cluster Major
@effect/experimental Major
@effect/opentelemetry Major
@effect/platform-browser Major
@effect/platform-bun Major
@effect/platform-node-shared Major
@effect/platform-node Major
@effect/platform Major
@effect/printer-ansi Major
@effect/printer Major
@effect/rpc-http Major
@effect/rpc Major
@effect/schema Major
@effect/sql-d1 Major
@effect/sql-drizzle Major
@effect/sql-kysely Major
@effect/sql-libsql Major
@effect/sql-mssql Major
@effect/sql-mysql2 Major
@effect/sql-pg Major
@effect/sql-sqlite-bun Major
@effect/sql-sqlite-node Major
@effect/sql-sqlite-react-native Major
@effect/sql-sqlite-wasm Major
@effect/sql Major
@effect/typeclass Major
@effect/vitest Major

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@evelant
Copy link
Contributor Author

evelant commented May 29, 2024

@mikearnaldi would love to get your feedback on this when you've got a moment. I think it's good to go but this is my first stab at adding a new module and I may have missed things.

@evelant evelant force-pushed the feat/tsubscriptionref branch 2 times, most recently from 90c23d6 to 6445e04 Compare June 23, 2024 14:12
@evelant
Copy link
Contributor Author

evelant commented Jun 23, 2024

removed some comments and debug stuff I accidentally left in internal

@mikearnaldi
Copy link
Member

Cc @tim-smart for a second review, I think I am fine with it albeit have some doubts about mixing stream with stm

@evelant
Copy link
Contributor Author

evelant commented Aug 15, 2024

I'm curious what doubts you have about mixing stream and stm? The use case leading me to implement this is when I need the synchronization capabilities of stm but also need the value-over-time for things like rendering UI.

@mikearnaldi
Copy link
Member

I'm curious what doubts you have about mixing stream and stm? The use case leading me to implement this is when I need the synchronization capabilities of stm but also need the value-over-time for things like rendering UI.

Namely subscribing gives you back a non transactional primitive (Stream) I think I'd prefer if subscribing gave you a TDequeue like TPubSub does:

export const subscribe: <A>(self: TPubSub<A>) => STM.STM<TQueue.TDequeue<A>>

@evelant
Copy link
Contributor Author

evelant commented Aug 18, 2024

Hmm interesting. I'm not sure what benefit that would offer. I can't think of a use case right now when I'd want a TDequeue instead of a Stream. Did you have something in mind?

@mikearnaldi
Copy link
Member

Hmm interesting. I'm not sure what benefit that would offer. I can't think of a use case right now when I'd want a TDequeue instead of a Stream. Did you have something in mind?

The TQueue interface has all methods returning STM so they can be used transactionally. It's strictly more powerful as you can derive a Stream from a TQueue but not the opposite

packages/effect/src/TQueue.ts Show resolved Hide resolved
packages/effect/src/TSubscriptionRef.ts Outdated Show resolved Hide resolved
packages/effect/src/TSubscriptionRef.ts Outdated Show resolved Hide resolved
@evelant
Copy link
Contributor Author

evelant commented Aug 27, 2024

I'll update the PR with the requested changes later this week, thanks for the feedback!

@evelant evelant force-pushed the feat/tsubscriptionref branch 2 times, most recently from 09e058a to 48ec3d9 Compare October 4, 2024 15:39
@github-actions github-actions bot changed the base branch from main to next-minor October 4, 2024 15:39
@github-actions github-actions bot force-pushed the next-minor branch 7 times, most recently from a50c2cf to d75140c Compare October 6, 2024 23:48
@mikearnaldi
Copy link
Member

@tim-smart I think this is now in a good state, pls have a final review, feel free to merge directly is all looks good

@evelant
Copy link
Contributor Author

evelant commented Oct 9, 2024

@mikearnaldi thanks for finishing this up!

* @since 3.10.0
* @category mutations
*/
export const changesStream: <A>(self: TSubscriptionRef<A>) => Stream.Stream<A, never, never> = internal.changesStream
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
export const changesStream: <A>(self: TSubscriptionRef<A>) => Stream.Stream<A, never, never> = internal.changesStream
export const changesStream: <A>(self: TSubscriptionRef<A>) => Stream.Stream<A> = internal.changesStream

@tim-smart tim-smart merged commit 2828b7f into Effect-TS:next-minor Oct 10, 2024
11 checks passed
@github-actions github-actions bot mentioned this pull request Oct 10, 2024
github-actions bot pushed a commit that referenced this pull request Oct 11, 2024
Co-authored-by: Michael Arnaldi <[email protected]>
Co-authored-by: Tim <[email protected]>
github-actions bot pushed a commit that referenced this pull request Oct 11, 2024
Co-authored-by: Michael Arnaldi <[email protected]>
Co-authored-by: Tim <[email protected]>
github-actions bot pushed a commit that referenced this pull request Oct 11, 2024
Co-authored-by: Michael Arnaldi <[email protected]>
Co-authored-by: Tim <[email protected]>
github-actions bot pushed a commit that referenced this pull request Oct 11, 2024
Co-authored-by: Michael Arnaldi <[email protected]>
Co-authored-by: Tim <[email protected]>
github-actions bot pushed a commit that referenced this pull request Oct 11, 2024
Co-authored-by: Michael Arnaldi <[email protected]>
Co-authored-by: Tim <[email protected]>
github-actions bot pushed a commit that referenced this pull request Oct 13, 2024
Co-authored-by: Michael Arnaldi <[email protected]>
Co-authored-by: Tim <[email protected]>
github-actions bot pushed a commit that referenced this pull request Oct 13, 2024
Co-authored-by: Michael Arnaldi <[email protected]>
Co-authored-by: Tim <[email protected]>
github-actions bot pushed a commit that referenced this pull request Oct 13, 2024
Co-authored-by: Michael Arnaldi <[email protected]>
Co-authored-by: Tim <[email protected]>
github-actions bot pushed a commit that referenced this pull request Oct 14, 2024
Co-authored-by: Michael Arnaldi <[email protected]>
Co-authored-by: Tim <[email protected]>
github-actions bot pushed a commit that referenced this pull request Oct 14, 2024
Co-authored-by: Michael Arnaldi <[email protected]>
Co-authored-by: Tim <[email protected]>
github-actions bot pushed a commit that referenced this pull request Oct 14, 2024
Co-authored-by: Michael Arnaldi <[email protected]>
Co-authored-by: Tim <[email protected]>
github-actions bot pushed a commit that referenced this pull request Oct 14, 2024
Co-authored-by: Michael Arnaldi <[email protected]>
Co-authored-by: Tim <[email protected]>
tim-smart added a commit that referenced this pull request Oct 17, 2024
Co-authored-by: Michael Arnaldi <[email protected]>
Co-authored-by: Tim <[email protected]>
github-actions bot pushed a commit that referenced this pull request Oct 17, 2024
Co-authored-by: Michael Arnaldi <[email protected]>
Co-authored-by: Tim <[email protected]>
github-actions bot pushed a commit that referenced this pull request Oct 17, 2024
Co-authored-by: Michael Arnaldi <[email protected]>
Co-authored-by: Tim <[email protected]>
github-actions bot pushed a commit that referenced this pull request Oct 20, 2024
Co-authored-by: Michael Arnaldi <[email protected]>
Co-authored-by: Tim <[email protected]>
github-actions bot pushed a commit that referenced this pull request Oct 21, 2024
Co-authored-by: Michael Arnaldi <[email protected]>
Co-authored-by: Tim <[email protected]>
tim-smart added a commit that referenced this pull request Oct 21, 2024
Co-authored-by: Michael Arnaldi <[email protected]>
Co-authored-by: Tim <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants