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

Expose Polka class #161

Closed
ghostdevv opened this issue Apr 7, 2021 · 9 comments
Closed

Expose Polka class #161

ghostdevv opened this issue Apr 7, 2021 · 9 comments

Comments

@ghostdevv
Copy link

Would it be possible to export the Polka class? Something I am writing would need this.

@talentlessguy
Copy link

Adding export keyword here

class Polka extends Router {
would work.

I think it's worth making a PR to next branch

@ghostdevv
Copy link
Author

I know, I was more asking if it's something luke would allow. I can make a pr anyway.

@lukeed
Copy link
Owner

lukeed commented May 25, 2021

It's more a question of why this is wanted

Traditionally my response to this question has been to not do it, since it exposes the opportunity for people to accidentally do some nasty extensions to Polka and then blame Polka itself.

@lukeed
Copy link
Owner

lukeed commented May 25, 2021

There's also already a PR for this: #103

@ghostdevv
Copy link
Author

It's more a question of why this is wanted

I wanted to extend Polka to add some features for my use case.

There's also already a PR for this: #103

Awesome

@lukeed
Copy link
Owner

lukeed commented May 27, 2021

I wanted to extend Polka to add some features for my use case

Right, to do what? What do you need to add?

@ghostdevv
Copy link
Author

Right, to do what? What do you need to add?

I was writing a router that lays on top of polka and extends it's base class to add new functionality, I ended up scraping that idea and instead binding it to polka instead, like this:

import polka from 'polka';
import { router } from 'x'; // Calling it x for this example

const server = polka();
const x = router(server);

@lukeed
Copy link
Owner

lukeed commented May 27, 2021

Yeah, I would just wrap it and add the functionality you're looking to add.

You can also instantiate a Polka app via polka() and then read out its constructor, extend it, and then reinstantiate your final class from there. It's ugly, but it's supposed to be. My original point still stands in that protecting the Polka class prevents users from accidentally breaking their applications, which wouldnt be hard to do since all internals are exposed.

Something like this:

const Polka = polka().constructor;

class MyRouter extends Polka {
  constructor(options) {
    super(options);
    // ...other stuff
  }
  foobar() {
    return 'foobar method';
  }
}

let app = new MyRouter({ ... });
app.foobar(); //=> "foobar method"
app.get('/users/:name', reply).listen(PORT);

Ugly, but visually obvious you're doing something off-the-path.
Same thing applies to app.foobar = function () { ... } IMO.

Here's a non-Polka, quick demo:

class Animal {
  walk() {
    return 'walking';
  }
}

class Cat extends Animal {
  #foo = 123;
  change(val) {
    this.#foo = val;
  }
  meow() {
    return `meowing: ${this.#foo}`;
  }
}

let foo = new Cat;
let Ctor = foo.constructor;

class Extra extends Ctor {
  extra() {
    return ('extra')
  }
}

let bar = new Extra();
console.log('meow#1', bar.meow());
//=> meow#1 meowing: 123
console.log('extra', bar.extra());
console.log('walk', bar.walk());

console.log('change', bar.change('asd'));
console.log('meow#2', bar.meow());
//=> meow#2 meowing: asd

Closing for now, as I still don't think this should be done.

@lukeed lukeed closed this as completed May 27, 2021
@ghostdevv
Copy link
Author

Thank you for the alternative, it doesn't matter if it's internally ugly as long as everything still works fine.

Closing for now, as I still don't think this should be done.

I am not quite sure how they could break it in this case as all the polka function does is return a instance of the class, people won't just accidentally extend the class and break something as they would have to know what they are trying to do to import the class in the first place let alone extend it

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

No branches or pull requests

3 participants