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

Don’t register storage event listener in Node.js environments #26

Closed
wants to merge 1 commit into from

Conversation

hanspagel
Copy link

This PR

  1. Checks if it’s running in a browser context.
  2. Only then registers the storage event listener in the LocalStoragePolyfill.

Merging this PR would make y-websocket run in Node.js environments, see yjs/y-websocket#51 yjs/y-websocket#55

Alternative solutions

  1. It doesn’t make sense to use the LocalStoragePolyfill in Node.js environments at all. A check to load the Polyfill only in a browser context would probably work too.
  2. Add for example node-localstorage to add support for localstorage to Node.js.

Let me know if you I can do anything to improve the PR. 💖

@dmonad dmonad closed this in a1202d4 Mar 21, 2021
@dmonad
Copy link
Owner

dmonad commented Mar 21, 2021

Hey @hanspagel ,

Thanks for the PR! I want to leave the option open to implement an actual storage polyfill for nodejs. So I went with a slightly different approach and added a global event listener storage.onChange(event => ..) that should be used instead. Hope this works for you. I closed the related issues in y-websocket already. Let me know if there are still issues.

The issue is fixed in lib0@v0.2.41.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants