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

Support subclassing Observable with non-class constructor functions. #1

Merged
merged 1 commit into from
Feb 2, 2021

Conversation

benjamn
Copy link
Member

@benjamn benjamn commented Feb 2, 2021

@benjamn benjamn self-assigned this Feb 2, 2021
function construct(Super, instance, subscriber) {
return typeof Reflect === 'object'
? Reflect.construct(Super, [subscriber], instance.constructor)
: Function.prototype.call.call(Super, instance, subscriber) || instance;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have to use Function.prototype.call.call(Super, ...) instead of Super.call(...), since Super is often the Observable constructor, and we do not want to call back into the Observable.call implementation above.

@benjamn benjamn force-pushed the allow-non-native-subclassing branch from 7895e15 to 26e8623 Compare February 2, 2021 21:12
@benjamn benjamn force-pushed the allow-non-native-subclassing branch from 26e8623 to 304255e Compare February 2, 2021 21:14
@benjamn benjamn merged commit ae559f5 into main Feb 2, 2021
@benjamn benjamn deleted the allow-non-native-subclassing branch February 2, 2021 21:16
benjamn added a commit that referenced this pull request Aug 24, 2021
We implemented these methods explicitly in PR #1, but then PR #105 went
back to compiling `Observable` down to ES5 constructor functions.

In other words, the current implementations of `Observable` (both
CommonJS and ESM) automatically have these static methods, and they work
just like `Function.prototype.{call,apply}`, because `Observable` is
compiled in both cases from a `class` to a constructor function.
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.

1 participant