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

Consider renaming MainDOMSource to DOMSource #951

Open
wclr opened this issue Sep 1, 2020 · 1 comment
Open

Consider renaming MainDOMSource to DOMSource #951

wclr opened this issue Sep 1, 2020 · 1 comment

Comments

@wclr
Copy link
Contributor

wclr commented Sep 1, 2020

There was a PR that introduced MainDOMSource, BodyDOMSource or DocumentDOMSource as variants of DOMSource union type (https://github.com/cyclejs/cyclejs/blob/master/dom/src/DOMSource.ts#L16)

Usually in the code, we use MainDOMSource as type of DOM source. And document or body sources are available from any normal DOM source are used as: DOM.select('document').events(...).

Explicit use of BodyDOMSource or DocumentDOMSource types is a very rare case only for specific components, and probably even more rare case would be expecting DOM source to be MainDOMSource | BodyDOMSource | DocumentDOMSource (which DOMSource now), even if so explicit statement of BodyDOMSource | DocumentDOMSource would be even more appropriate.

So why this common DOM source it is called MainDOMSource, why not CommonDOMSource or ElementDOMSource, prefix Main probably the shortest option and has some sense but still seems unnatural and contrived here.

So I propose to remove/deprecate MainDOMSource and rename it to DOMSource, removing union type, the rationale for this is to make reference to commonly used DOM source type more simple and conventional, as there is no real practical reason to have a union type for all kind of sources (DOMSource union type itself actually is not used in the driver's code anywhere). So DOMSource would mean normal commonly used dom source which supports isolation and etc., and there will be specific two types BodyDOMSource and DocumentDOMSource which will be explicitly used when needed.

So this is my current perspective on this situation with quite ugly/contrived name MainDOMSource, but maybe I missed something.

@staltz
Copy link
Member

staltz commented Sep 1, 2020

This is something we can think about, for the neocycle's DOM plugin.

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

No branches or pull requests

2 participants