globalThis incorrectly pollyfilled #23

Open
opened 2024-08-27 19:30:43 +00:00 by gilbert · 3 comments
Member

In registry.ts, globalThis is incorrectly polyfilled, which causes it to choose a different global instead of an already-present globalThis.

Namely, the act of declaring var globalThis shadows the actual globalThis, causing globalThis to be undefined within the IIFE.

This becomes an issue in node.js server-side rendering environments where window is polyfilled, and node's globalThis has different functions than window does.

Suggested fixes:

  1. Rename the variable to something else, or
  2. Just use globalThis without attempting to polyfill. LTS versions of node support it, and versions as far back as v12 (released in 2019).
In [registry.ts](https://git.vdb.to/cerc-io/registry-sdk/src/commit/f93a29785704e8c154a4256e8afd6143a8b7efea/src/proto/cerc/registry/v1/registry.ts#L1165-L1171), globalThis is incorrectly polyfilled, which causes it to choose a different global instead of an already-present `globalThis`. Namely, the act of declaring `var globalThis` shadows the actual `globalThis`, causing `globalThis` to be undefined within the IIFE. This becomes an issue in node.js server-side rendering environments where `window` is polyfilled, and node's `globalThis` has different functions than `window` does. Suggested fixes: 1. Rename the variable to something else, or 2. Just use `globalThis` without attempting to polyfill. LTS versions of node support it, and versions as far back as v12 (released in 2019).
Author
Member

See this code example for an illustration of the issue.

See [this code example](https://jsbin.com/cotehupazu/edit?js,console) for an illustration of the issue.
Member

re-opening b/c this patch needs to be applied to main. cut patch release to unblock progress on snowball

re-opening b/c this patch needs to be applied to main. cut patch release to unblock progress on snowball
zramsay reopened this issue 2024-08-27 23:39:47 +00:00
ashwin was assigned by zramsay 2024-08-27 23:39:56 +00:00
Member

re-opening b/c this patch needs to be applied to main. cut patch release to unblock progress on snowball

Handled in #25
Included in release v0.2.7

> re-opening b/c this patch needs to be applied to main. cut patch release to unblock progress on snowball Handled in #25 Included in release [v0.2.7](https://git.vdb.to/cerc-io/registry-sdk/releases/tag/v0.2.7)
Sign in to join this conversation.
No Milestone
No project
No Assignees
3 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: cerc-io/registry-sdk#23
No description provided.