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

move ManagedRuntime.TypeId to fix circular imports #3682

Conversation

leonitousconforti
Copy link
Contributor

Type

  • Refactor
  • Feature
  • Bug Fix
  • Optimization
  • Documentation Update

Description

Right now, there is two typeId definitions for effect/ManagedRuntime, one in layer and one in managedRuntime. If you change one of them then everything still type-checks, but it would not be correct. Would this be an appropriate solution? Didn't make a public MemoMap file because I didn't think it warranted changing the public Layer file

Related

  • Related Issue #
  • Closes #

Copy link

changeset-bot bot commented Sep 25, 2024

🦋 Changeset detected

Latest commit: 145169c

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

This PR includes changesets to release 32 packages
Name Type
effect Patch
@effect/cli Patch
@effect/cluster-browser Patch
@effect/cluster-node Patch
@effect/cluster-workflow Patch
@effect/cluster 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/sql-d1 Patch
@effect/sql-drizzle Patch
@effect/sql-kysely Patch
@effect/sql-libsql Patch
@effect/sql-mssql Patch
@effect/sql-mysql2 Patch
@effect/sql-pg Patch
@effect/sql-sqlite-bun Patch
@effect/sql-sqlite-node Patch
@effect/sql-sqlite-react-native Patch
@effect/sql-sqlite-wasm Patch
@effect/sql Patch
@effect/typeclass Patch
@effect/vitest 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

@leonitousconforti leonitousconforti marked this pull request as ready for review September 25, 2024 06:08
@github-actions github-actions bot force-pushed the next-minor branch 2 times, most recently from 77f5f53 to cde84e8 Compare September 25, 2024 07:50
@mikearnaldi
Copy link
Member

/rebase

@github-actions github-actions bot force-pushed the fix-layer-managed-runtime-circular-imports branch from 857f8bb to dbffd3d Compare September 25, 2024 08:25
@tim-smart
Copy link
Member

tim-smart commented Sep 25, 2024

I think in this case we could just move the ManagedRuntime.TypeId to a internal/managedRuntime/circular.ts file.

We can keep memo map in the layer module.

@leonitousconforti leonitousconforti force-pushed the fix-layer-managed-runtime-circular-imports branch from dbffd3d to 07118d5 Compare September 25, 2024 21:49
@leonitousconforti leonitousconforti changed the title Add internal memoMap file to fix layer and managed runtime circular imports Create managedRuntime/circular.ts Sep 25, 2024
@tim-smart tim-smart changed the title Create managedRuntime/circular.ts move ManagedRuntime.TypeId to fix circular imports Sep 26, 2024
@tim-smart tim-smart merged commit 687e926 into Effect-TS:next-minor Sep 26, 2024
11 checks passed
@tim-smart tim-smart mentioned this pull request Sep 26, 2024
github-actions bot pushed a commit that referenced this pull request Sep 26, 2024
github-actions bot pushed a commit that referenced this pull request Sep 26, 2024
github-actions bot pushed a commit that referenced this pull request Sep 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants