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

Allow subclass constructors without super() call #7285

Open
justinfagnani opened this issue Feb 28, 2016 · 21 comments
Open

Allow subclass constructors without super() call #7285

justinfagnani opened this issue Feb 28, 2016 · 21 comments
Labels
Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript

Comments

@justinfagnani
Copy link

TypeScript Version:

1.8.0

Code

Custom element classes often have constructors that just return document.createElement(), since HTMLElement doesn't have a callable constructor, but this generates an error in TypeScript:

class MyElement extends HTMLElement {
  constructor() {
    return document.createElement('my-element');
  }
}

In general, it is completely valid ES2015 to not call super(), as long as you don't access this.

@nippur72
Copy link

Something similar was discussed in #5910

@xLama
Copy link

xLama commented Feb 29, 2016

I know this is a very ugly solution but It works.

class MyElement extends HTMLElement {

    constructor() {
        try{
            super();
        }catch(e){
        }finally{
            return document.createElement('my-element');
        }
    }
}

Or without catch

class MyElement extends HTMLElement {

    constructor() {
        try{
            super();
        }finally{
            return document.createElement('my-element');
        }
    }
}

class MyElement extends HTMLElement {
    constructor(element : string) {
        try{
            super();
        }finally{
            return document.createElement(element);
        }
    }
}

alert(new MyElement("div"));

@justinfagnani
Copy link
Author

Apparently TS's super() check isn't too robust, so I just did this:

constructor() {
  if (1 < 0) { // edit, this used to be 1 < 0, @xLama is right :)
    super();
  }
  return document.createElement('my-element');
}

@xLama
Copy link

xLama commented Feb 29, 2016

I think it dows not work because it call super constructor. 1 is always greater than 0. Maybe you wanted to write 1 < 0.

I think that code will beacome deprecated when unreachable code detection improves.

this does not work


class MyElement extends HTMLElement {
    constructor() {
        if (false){
          super();
        }
        return document.createElement();
    }
}

this it does

class MyElement extends HTMLElement {
    constructor() {
        if (0){
          super();
        }
        return document.createElement();
    }
}

@justinfagnani
Copy link
Author

Oops, I meant the other way! Editing...

@justinfagnani
Copy link
Author

@xLama there will always be some way to beat compile-time evaluation... it'll just get uglier over time.

@RyanCavanaugh RyanCavanaugh added the Needs More Info The issue still hasn't been fully clarified label Feb 29, 2016
@RyanCavanaugh
Copy link
Member

What's the use of a class like this? Why not just have a factory function?

It seems crazy to have MyElement such that (new MyElement()) instanceof MyElement is false

@justinfagnani
Copy link
Author

(new MyElement()) instanceof MyElement will be true. document.createElement('my-element') will create an instance of MyElement.

The use, in this case, is that users want constructors, even before the built-in HTMLElement subclasses get callable constructors.

In other cases, a constructor could return a subtype, or return a cached instance.

@RyanCavanaugh
Copy link
Member

That's not what I'm seeing in Firefox nightly -- maybe a bug on their side?

@justinfagnani
Copy link
Author

Firefox custom element support is behind a flag. You need to enable dom.webcomponents.enabled in about:config. Or try on Chrome.

@justinfagnani
Copy link
Author

Here's an example that works in Chrome: http://jsbin.com/xivutarobo/edit?html,console,output

With polyfills is also works on other browsers without flags.

@justinfagnani
Copy link
Author

Adding the code here, for convenience:

    class MyElement extends HTMLElement {
      constructor() {
        return document.createElement('my-element');
      }

      createdCallback() {
        console.log('created my-element');
      }
    }
    document.registerElement('my-element', MyElement);
    let el = new MyElement();
    console.log(el instanceof MyElement); // true

@justinfagnani
Copy link
Author

Is more info still needed, or was this enough?

@RyanCavanaugh RyanCavanaugh added Suggestion An idea for TypeScript In Discussion Not yet reached consensus and removed Needs More Info The issue still hasn't been fully clarified labels Mar 4, 2016
@RyanCavanaugh
Copy link
Member

I think this is enough.

For the vast majority of ES6 classes, not calling super is a big runtime error. We'd need to figure out how to still give an error in those cases while still allowing this.

@justinfagnani
Copy link
Author

One big sign that superisn't necessary in the case of factory constructors is the presence of a return statement. Along with no access to this, it might be enough.

Otherwise, is there any existing warning suppression mechanism in TypeScript?

@aluanhaddad
Copy link
Contributor

@justinfagnani If you are returning a value, not accessing this, and not calling super, why is it defined as a class. What exactly do you mean by 'factory constructors'? I know what they are in dart, but in JavaScript/TypeScript people usually speak in terms of factories or constructors.

@justinfagnani
Copy link
Author

First, it's just valid JavaScript that can type check, so it seems like it should be supported by a superset of JavaScript. As for use cases, the custom element use case is my primary concern. HTMLElement doesn't have a callable constructor yet, so extending it requires not calling super().

As for the term "factory constructor", yes Dart explicitly has this concept, but JavaScript already supports constructors returning something other than this. I think such constructors can reasonably be called factory constructors, instead of "constructors that return something other than this".

@DanielRosenwasser DanielRosenwasser added the Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. label Jul 5, 2016
@DanielRosenwasser
Copy link
Member

Given that HTMLElement's constructor never actually returns, it should actually return never. I'm thinking that

  1. It should be allowed to extend something which has a construct signature of type never.
  2. If a construct signature of some value's type returns never, then a super call is not needed if extending from that value.
  3. If there is a super() call, it should be an error to access this before super (which is currently the case anyway).

@justinfagnani
Copy link
Author

The examples here, specifically regarding subclasses of HTMLElement are a bit out of date.
HTMLElement is now specified to return a value. See: https://html.spec.whatwg.org/multipage/dom.html#html-element-constructors in particular step 11.

@justinfagnani
Copy link
Author

#7574 is much, much more important to the Custom Elements use cases now.

@SephReed
Copy link

SephReed commented Sep 15, 2018

Is there any way this check could be put behind a flag? Being able to disregard super is incredibly useful for evolutions.

In my case I want to evolve an event, so I have something like this:

class SuperEvent extends Event {
  public superHeroIdentity: string;

  constructor(event: Event) {
    super(); // really trying to get rid of this line
    (event as any).superHeroIdentity = "Megavent";
    return event;
  }
}

const boringEvent = new Event("doThing").
const superEvent = new SuperEvent(boringEvent);
console.log(superEvent); // Megavent

I know evolution's aren't a wide spread concept in the programming world just yet, but they're incredibly useful and one of the best things about javascript. All of this is valid javascript, so it would be really nice if typescript was capable of it as well. Also I recognize how this breaks strict typing, but I'm not really trying to compile wasm here.

Also, in this case

MyElement such that (new MyElement()) instanceof MyElement is [true]

@RyanCavanaugh RyanCavanaugh removed the In Discussion Not yet reached consensus label Mar 7, 2019
@RyanCavanaugh RyanCavanaugh removed In Discussion Not yet reached consensus labels Mar 7, 2019
Offroaders123 added a commit to Offroaders123/NBTify that referenced this issue May 9, 2023
This is the custom logging behavior mentioned in the previous commit, it's super neat! I've wanted to learn how to do this for a while now, and I ended up finding the right documentation which described everything I needed to do in order to get it to work.

It's a Node-specific thing, so this is meant for that.

Essentially, you can customize all of the ways that Node will log a given object, and you can specify things like the actual text that comes out in the log, what color it should be, and things like that. Using that, I essentially made it so the content of the `NBTData.prototype.data` property is shown on the `NBTData` log itself, rather than all of the properties present on `NBTData`. The only thing I don't like about this is that it could make NBTify users think that the `NBTData` object itself directly holds the keys/values, when it's actually the `data` property that has them. I think this does look nicer in terms of the console output though.

Here's some demo code to set it up:

```ts
// https://stackoverflow.com/questions/10729276/how-can-i-get-the-full-object-in-node-jss-console-log-rather-than-object
// https://nodejs.org/api/util.html#utilinspectcustom
// nodejs/node#35956

import type { inspect as inspectFn, InspectOptions } from "node:util";

export class Byte extends Number {
  constructor(value: number) {
    super(value);
  }

  get [Symbol.toStringTag]() {
    return "Byte" as const;
  }
}

export class Short extends Number {
  constructor(value: number) {
    super(value);
  }

  get [Symbol.toStringTag]() {
    return "Short" as const;
  }

  // [inspect.custom]() {
  [Symbol.for("nodejs.util.inspect.custom")](depth: number, options: InspectOptions, inspect: typeof inspectFn) {
    return `${this[Symbol.toStringTag]} { ${inspect(this.valueOf(),{ colors: true })} }`;
  }
}

console.log(Number(16));
console.log(new Number(42));
console.log(new Byte(25));
console.log(new Short(83));
console.log(new Uint8Array(12));
```

After discovering this, I happened to look into the BigInt primitive wrapper extending class problem (it's not actually a constructor, so you can't extend it using the `class` syntax, 'kind of', I've come to learn and find out :) ).

Not sure which way I will take this, as to whether I will move the `LongTag` type over to it's own primitive wrapper class (that's been keeping the API in limbo, in-between either all primitive wrapper classes, or only primitives themselves). I think it's been a nice sweet spot to rely on types only to do things, and to use library module-specific functions to deal with the primitives, instead of doing everything with wrapper objects and methods. Now that I have this demo working though, I think I want to try out the primitive wrapper object syntax again. Not sure how much I like it though. It does at least kind of 'seem' like it could make the library simpler, but I'm not so sure about that, having learned more of the downfalls with OO programming.

```ts
// tc39/proposal-bigint#98
// https://stackoverflow.com/questions/3350215/what-is-returned-from-a-constructor
// microsoft/TypeScript#7285

import type { InspectOptionsStylized } from "node:util";

declare global {
  interface BigIntConstructor {
    new (value: bigint | boolean | number | string): BigInt;
  }
}

export class Long<T extends bigint = bigint> extends BigInt {
  // @ts-expect-error
  constructor(value: T) {
    return Object.setPrototypeOf(Object(BigInt(value)),Long.prototype);
  }

  override valueOf() {
    return super.valueOf();
  }

  override toString() {
    return `${this.valueOf()}l`;
  }

  // @ts-expect-error
  get [Symbol.toStringTag]() {
    return "Long" as const;
  }

  [Symbol.for("nodejs.util.inspect.custom")](_: number, { stylize }: InspectOptionsStylized) {
    return stylize(this.toString(),"bigint");
  }
}

const myThing = new Long(25n);
console.log(myThing,25n);
console.log(myThing instanceof BigInt);
```

Yeah, so I'm trying to decide whether to move the logic for doing things into wrapper classes, or to try and keep raw primitives wherever possible. This could essentially strengthen NBTify's associations between the tag types, and the classes/primitives themselves, which I think is what I like out of this.

For example, the `getTagType()` function currently uses a mix of `typeof` and `instanceof` checks to see if that value is one of the NBTify primitives/wrapper objects. If everything is made with wrapper objects, you could instead just use `instanceof` directly, and check if the value is an instanceof the given 'NBTify primitive', essentially any of the wrapper objects for all of the tag types, in the case that everything would be primitive wrapper objects. This could take some of the logic out of the type checking, and into the data structures themselves, or something like that.

I think one of the other things that I liked about the idea for primitive wrapper objects too, is that you could add your own `toString()` methods, and they could narrow down to plain SNBT, which NBTify could them parse directly. You might not need to have a `stringify()` function; at least, you could even just wrap the `toString()` of the root `CompoundTag` to be returned from `stringify()`, and the recursive nature of the data tree's `toString()` methods would manage the stringification of the tree, rather than the `stringify()` function itself. This moves the stringification logic out of the function, and onto the objects to handle themselves.

But for the already-existing wrapper objects, like the TypedArrays, I kind of don't like having to call a custom NBTify constructor just to create that NBT primitive type. It's only necessary to do for the single-number types, because they don't have JavaScript equivalents or alternatives.

That last paragraph made me think some more, what would JavaScript do? What's the best scenario here?

If this were JavaScript's realm, it would probably have the primitive types for these built-in. And if they are primitive types, they wouldn't be used by default as wrapper objects, you'd use them as the primitives themselves, just like how `string`, `number`, `bigint`, `boolean`, and `symbol` work right now. They may have wrapper objects, but you don't actually use them in your regular old code like that. You just use their primitives. If that's the case, and JavaScript *did* have those primitives built-in, then I wouldn't be using primitive wrapper objects for everything. Seeing it that way, it's obvious.

The best scenario would be for JavaScript to support the lower-level number primitives, `int8`, `int16`, `int32`, and `float32`. That would allow me to remove the number wrapper types, because the language and runtime itself could distinguish the values of each as different from each other. That's been my main reason for choosing to make wrapper objects for those. I didn't want to lose any type information from the NBT tree when parsing them as numbers in JavaScript. The `number` type itself can handle all of those values, they just can't be distinguished from one another. NBTify could always just assume what it should be, based on it's size, but this isn't safe, because this would essentially just coalesce everything to the smallest number bracket that it could fit into (most of the time `ByteTag` in NBT). And trying to guess the type is harder than already having it provided to you, by the source data itself. And if the primitives were available in JavaScript, that would be the simplest option. The only reason this one is in similar complexity to guessing the data type container, is because I have to re-create the data containers in JavaScript, because the primitives aren't (yet?) available.

So, with that whole thing in mind, I think I should likely stay away from the full primitive wrapper object route, because while it does seem to break up the logic into nice components, it inherently prevents you from using the data as primitives themselves, and it requires you to use objects to wrap the data itself, while it does break it up into nice containing blocks.

Ok, that was a lot of writing. Wasn't sure if I covered everything, but I'm happy how much I did. Listening to parts of Marco's new 'Their Colors Fade' album today, not sure how many times I've listened through all the different parts just today. I really like it a lot. I'm hearing more parts of his earlier albums that I hadn't noticed before, and it feels like a nice reference to the past. It definitely feels like a whole new concept and vibe, I really like it. The songs that are newer to me are really starting to catch on now. I knew a good few of them from during the time he was posting about working on the album online, so I think that's why the album has caught on so quick for me. It was familiar in understanding the vibe, but now it says something more specific as a whole when hearing them all together as one piece. He was right about the track list order, it did turn out perfect.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Proposal This issue needs a plan that clarifies the finer details of how it could be implemented. Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

7 participants