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

[MMB-317], [MMB-318] — Remove the LockAttributes type #214

Merged
merged 1 commit into from
Oct 16, 2023

Conversation

lawrence-forooghian
Copy link
Collaborator

@lawrence-forooghian lawrence-forooghian commented Oct 9, 2023

Resolves MMB-317 and MMB-318.

I have chosen to simply remove the LockAttributes class and replace it with Record<string, unknown>, which is what we use elsewhere for representing arbitrary key-value pairs in the public API of the SDK.

See ably/docs#2023 for corresponding documentation update.

Resolves MMB-317:

> During the locking work we added an export for LockAttributes. This broke the CDN build because rollup will only create a Spaces global constructor if there is a single export from it’s entry file. Instead it exports a fake ES6 module:
>
> Spaces global === { default: Spaces constructor, LockAttributes }
>
> instead of:
>
> Spaces global === Spaces constructor

and resolves MMB-318:

> In docs, we show a way to get values from LockAttributes
> https://ably.com/docs/spaces/locking#subscribe in the subscriber. This
> is however incorrect, because attributes on the receiving side is never
> a Map, but an object.

I have chosen to simply remove the LockAttributes class and replace it
with Record<string, unknown>, which is what we use elsewhere for
representing arbitrary key-value pairs in the public API of the SDK.
@lawrence-forooghian lawrence-forooghian merged commit e0f7dde into main Oct 16, 2023
5 checks passed
@lawrence-forooghian lawrence-forooghian deleted the remove-LockAttributes branch October 16, 2023 11:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants