web4more / html-navigator

Navigator api for node.js
https://npmjs.com/package/node-navigator
MIT License
3 stars 0 forks source link

Move burden of `.applyToClass()` from source mixin to target interface #10

Open jcbhmr opened 1 year ago

jcbhmr commented 1 year ago

Specifically, I think that instead of

https://github.com/skdhg/node-navigator/blob/1fbc16049da715f270eba1baf857a914c602ba6e/src/NavigatorConcurrentHardware.ts#L9-L19

It should be something like:

class Navigator {}
const s1 = Object.getOwnPropertyDescriptors(NavigatorConcurrentHardware)
delete s1.name
delete s1.length
Object.defineProperties(Navigator, s1)
const p1 = Object.getOwnPropertyDescriptors(NavigatorConcurrentHardware.prototype)
delete p1.constructor
delete p1[Symbol.toStringTag]
Object.defineProperties(Navigator.prototype, p1)
// etc for each interface to add to Navigator

I think this would also highlight to other polyfills like a theoretical NavigatorStorage polyfill https://storage.spec.whatwg.org/#idl-index that this is how you apply a mixin to an existing class/interface.

For instance, here's an excerpt from https://github.com/skdhg/node-navigator/pull/6

Here's an example of how you might write a polyfill to mixin your MyNavigatorMixin to an existing Navigator implementation (like this package!):

// Expect that the user already has either:
// a) A minimal 'globalThis.Navigator = class {}' and
//    'globalThis.navigator = new Navigator()' implementation
// b) A polyfill that implements the 'Navigator' interface (like this
//    node-navigator package)
declare global {
  class Navigator {}
  var navigator: Navigator;
}
const s = Object.getOwnPropertyDescriptors(MyNavigatorMixin);
delete s.name;
delete s.length;
Object.defineProperties(Navigator, s);
const p = Object.getOwnPropertyDescriptors(MyNavigatorMixin.prototype);
delete p.constructor;
delete p[Symbol.toStringTag];
Object.defineProperties(Navigator.prototype, p);
// šŸ¤š Only hook into the constructor if absolutely necessary!
Navigator = new Proxy(Navigator, {
  construct(target, args, newTarget) {
    const o = Reflect.construct(target, args, newTarget);
    MyNavigatorMixin.call(o);
    return o;
  },
});
// Manually call on existing instances
MyNavigatorMixin.call(navigator);

@skdhg do you think this is a good idea? Thoughts?

twlite commented 1 year ago

Yeah a proxy is definitely better than my old implementation