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

Avoid putting symbols in global to fix incompatibility with Temporal Sandbox. #4196

Merged
merged 5 commits into from
Dec 27, 2024

Conversation

mikearnaldi
Copy link
Member

No description provided.

@mikearnaldi mikearnaldi marked this pull request as draft December 26, 2024 20:51
Copy link

changeset-bot bot commented Dec 26, 2024

🦋 Changeset detected

Latest commit: 13af68e

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

This PR includes changesets to release 35 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/sql-clickhouse 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-do 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
@effect/ai Patch
@effect/ai-openai 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

@@ -46,6 +42,12 @@ const globalStore = (globalThis as any)[globalStoreId] as Map<unknown, any>
* @since 2.0.0
*/
export const globalValue = <A>(id: unknown, compute: () => A): A => {
if (!globalStore) {
// @ts-expect-error
globalThis[globalStoreId] = new Map()
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
globalThis[globalStoreId] = new Map()
globalThis[globalStoreId] ??= new Map()

Not sure if we want to overwrite if it already exists.

Copy link
Member Author

Choose a reason for hiding this comment

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

how can it already exist if globalStore is undefined?

@izakfilmalter
Copy link

This pr is lit. Thank you @mikearnaldi and @mjameswh for helping me and getting this resolved.

@mikearnaldi mikearnaldi marked this pull request as ready for review December 27, 2024 12:16
@mikearnaldi mikearnaldi merged commit 39db211 into main Dec 27, 2024
12 checks passed
@mikearnaldi mikearnaldi deleted the bugfix/avoid-symbols-in-global branch December 27, 2024 13:00
@github-actions github-actions bot mentioned this pull request Dec 27, 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