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

Allow custom global (window) #116

Closed
wants to merge 6 commits into from
Closed

Conversation

anilanar
Copy link

@anilanar anilanar commented Oct 5, 2020

This is something we will use @userlike, not sure if you would be interested in this which is a non-breaking change. If so, we can work on merging this.

It introduces const createResizeObserver: (global: Window) => ResizeObserverCls and does some changes on the scheduler so that it is less "global". There are other minor changes such as ResizeObserverCls as an interface.

Our use case for this is observing changes inside an iframe.

I'm already aware that you have plans to introduce "observed root list" to have better shadow dom support. But until then, perhaps this is not a bad solution because this:

  1. is a non breaking change
  2. doesn't really conflict with such plans
  3. is already an improvement for certain use cases
  4. createResizeObserver(global: Window): ResizeObserverCls API surface is easily maintainable long-term.

(Sorry for formatting, prettier user here. Turn "whitespace changes" off. Can undo formatting if we agree on merging this).

Also, you can test it with @anilanar/resize-observer if you wish, which is published on npm.

@TremayneChrist
Copy link
Member

Hi @anilanar thanks for the submission!

For me, this is a bit of 'secret' back door entry into creating ROs and once this is published there is no turning back without breaking changes. This would also create multiple schedulers per controller which could impact performance quite a bit.

What I'd like to do, is suggest to keep this PR open for reference - I think there is some interesting things in there which could be useful. I'd also like to keep in contact and maybe test early release of Shadow DOM & Multi Document support with you?

@anilanar
Copy link
Author

anilanar commented Oct 5, 2020

Sounds good to me.

Also closing the PR but still continuing the conversation here is an option, I suppose.

Just ping me whenever you need help testing multi-document feature.

@TremayneChrist
Copy link
Member

Related to #42

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