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

Why are we using Promises here? #6

Closed
tabatkins opened this issue Aug 31, 2023 · 1 comment
Closed

Why are we using Promises here? #6

tabatkins opened this issue Aug 31, 2023 · 1 comment

Comments

@tabatkins
Copy link

I'm not certain why Promises are involved here. I understand that these are writing to what is likely an async storage, but that doesn't necessarily have to mean that the author-facing API is async, especially if it's solely used to track the overrides themselves.

That is, setting navigator.preference.colorScheme = "dark"; can be done synchronously; the preference object immediately reflects the "dark" value. But it will affect the actual MQ asynchronously, and authors can listen for that change with an appropriate async API.

This has several benefits:

  1. It avoids race conditions with several things overriding the same preference. Each thing will be able to see the preceding override already.
  2. It lets us give an immediate error if the value is set incorrectly, rather than requiring the author to handle a promise rejection.
  3. It lets us cut out the entire method-based interaction mode, letting us just use attributes (as in Being Map-ish seems a little overkill #4).

If we want to allow authors to see the "real" value of the MQ, we could add an async method to this API. We could probably just bake it into the getSupported() method, adding a currentValue attribute, since that method is already async.

lukewarlow added a commit that referenced this issue Aug 31, 2023
lukewarlow added a commit that referenced this issue Aug 31, 2023
lukewarlow added a commit that referenced this issue Aug 31, 2023
lukewarlow added a commit that referenced this issue Aug 31, 2023
@lukewarlow
Copy link
Collaborator

Thanks for the feedback! I've hopefully addressed the issues you've raised and have opened #8 to discuss the iframe behaviour in more detail.

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

No branches or pull requests

2 participants