-
-
Notifications
You must be signed in to change notification settings - Fork 421
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
MockedDOMSource is not assignable to DOMSource #869
Comments
Thanks for reporting this, @ryota-ka, I was fearing that this use case could appear as a breaking change, and it indeed did. The move to union type was a clever way of allowing us to "delete" types through TypeScript's "type ternary condition". I understand your need for an interface. @jvanbruegge Perhaps we could have some internal type used |
Is there any workaround for this or should I downgrade to a previous version to use mockDOMSource in a TypeScript project ? |
…MSource ISSUES CLOSED: cyclejs#869
@staltz @jvanbruegge While investigating this (after a year!), I've noticed this is a problem caused by the variance of I found that @jvanbruegge has supported TypeScript's strict mode in 90645d6, but until the time we have {co,conta}variant subtypes of My commit fixing this is here: ryota-ka@e8532c9 |
Waiting for the fix, I've been doing the following : export function intent(DOM: DOMSource | MockedDOMSource): Actions {
// https://github.com/cyclejs/cyclejs/issues/869
if (DOM instanceof MockedDOMSource) {
return {
addOne$: DOM.select(".add")
.events("click")
.mapTo(null),
removeOne$: DOM.select(".remove")
.events("click")
.mapTo(null)
};
} else {
return {
addOne$: DOM.select(".add")
.events("click")
.mapTo(null),
removeOne$: DOM.select(".remove")
.events("click")
.mapTo(null)
};
}
} This is quite ugly, is there a better way ? |
Sadly not at the moment. This won't be an issue in the new version, but that will still take a bit of time. Another workaround (what I've been doing) is in the tests type MockedDOMSource as |
The release of
@cycle/dom
v22.2.0 seems to have resulted in introducing a breaking change, though it's a minor version bump.In 90645d6,
DOMSource
has been rewritten to a union type, andMockedDOMSource
is intentionally excluded from this union.cyclejs/dom/src/DOMSource.ts
Lines 14 to 16 in c0bb763
Before this commit, it was defined as an interface, which allowed us to assign
MockedDOMSource
toDOMSource
.cyclejs/dom/src/DOMSource.ts
Lines 11 to 23 in 79344a4
As a result,
MockDOMSource
is no longer assignable toDOMSource
.Additionally, it seems that we don't have any precise types to which both
DOMSource
andMockedDOMSource
can be assign.Of course we can define something like
type T = DOMSource | MockedDOMSource
, but this definitiontsc
will claim as following (as oftypescript
v3.2.2).To me, it appears that it is not a very good idea to define
DOMSource
as a union type, as TypeScript's inference over structual subtypes are not strong enough. (Or can it be possibly fixed with better typings?)If possible, it would be great that
DOMSource
is defined as an interface again, with enabling TypeScript's strict mode.Thanks!
The text was updated successfully, but these errors were encountered: