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 Effect.Tag to simplify access to service #2225

Merged
merged 11 commits into from
Mar 2, 2024
Merged

Conversation

mikearnaldi
Copy link
Member

No description provided.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Copy link

changeset-bot bot commented Feb 29, 2024

🦋 Changeset detected

Latest commit: f8aba10

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

This PR includes changesets to release 15 packages
Name Type
effect Patch
@effect/cli Patch
@effect/experimental Patch
@effect/opentelemetry Patch
@effect/platform-browser Patch
@effect/platform-bun Patch
@effect/platform-node-shared Patch
@effect/platform-node Patch
@effect/platform Patch
@effect/printer-ansi Patch
@effect/printer Patch
@effect/rpc-http Patch
@effect/rpc Patch
@effect/schema Patch
@effect/typeclass Patch

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

TagClass["$"] = new Proxy({}, {
get(_target: any, prop: any, _receiver) {
// @ts-expect-error
return core.andThen(TagClass, (s) => s[prop])
Copy link
Member

Choose a reason for hiding this comment

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

maybe cache?

Copy link
Member Author

Choose a reason for hiding this comment

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

caching proxy leads to Max Call Stack

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member Author

Choose a reason for hiding this comment

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

I think this would be slower, not sure, the issue caching would try to prevent is proxy access but proxy is accessed anyway, for the rest a cache lookup or a single function call have comparable overhead

Copy link
Member

Choose a reason for hiding this comment

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

my point was more like garbage, creating a new function instead of lookup and return old. at least for the function case. but sounds like micro optimisations, especially considering all the functions we create and destroy on our side of the pond :D

@patroza
Copy link
Member

patroza commented Feb 29, 2024

@mikearnaldi is it intentional that the $ shape is not filtering out functions? This way there are dupes with different signatures.

like this would do: [k in keyof Shape as Shape[k] extends (...args: Array<any>) => any ? never : k] (inverse of the function one)

@mikearnaldi
Copy link
Member Author

@mikearnaldi is it intentional that the $ shape is not filtering out functions? This way there are dupes with different signatures.

like this would do: [k in keyof Shape as Shape[k] extends (...args: Array<any>) => any ? never : k] (inverse of the function one)

It is intentional, if the signatures are not the same they are not dupes. Every member is available to be accessed as a constant

@patroza
Copy link
Member

patroza commented Mar 1, 2024

It is intentional, if the signatures are not the same they are not dupes. Every member is available to be accessed as a constant

but
Screenshot 2024-03-01 at 16 11 19

@mikearnaldi
Copy link
Member Author

It is intentional, if the signatures are not the same they are not dupes. Every member is available to be accessed as a constant

but Screenshot 2024-03-01 at 16 11 19

to access a function and use it later for example

@mikearnaldi
Copy link
Member Author

it's of arguable importance though

@patroza
Copy link
Member

patroza commented Mar 1, 2024

it's of arguable importance though

yea, if you don't access it for use, it's kinda better to consume the service. rn it's kinda just confusing imo

@patroza
Copy link
Member

patroza commented Mar 2, 2024

@mikearnaldi
readonly doStuff: (...strings: readonly string[]) => Effect.Effect<string>
and
readonly doStuff: (...strings: ReadonlyArray<string>) => Effect.Effect<string>

infer to never

@mikearnaldi mikearnaldi changed the title Add Effect.AccessTag to simplify access to service Add Effect.Tag to simplify access to service Mar 2, 2024
@mikearnaldi mikearnaldi merged commit ac41d84 into main Mar 2, 2024
12 checks passed
@mikearnaldi mikearnaldi deleted the feat/access-tag branch March 2, 2024 16:59
@github-actions github-actions bot mentioned this pull request Mar 1, 2024
patroza pushed a commit to patroza/effect that referenced this pull request Mar 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

2 participants