Improved JSDoc comments for type correctness. #1161
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ | |
| /coverage | ||
| package-lock.json | ||
| /test/ts/**/*.js | ||
| .*.swp | ||
|
|
||
| # Additional bundles | ||
| /devtools.js | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,13 +33,13 @@ export function createComponent(Ctor, props, context) { | |
| inst; | ||
|
|
||
| if (Ctor.prototype && Ctor.prototype.render) { | ||
| inst = new /** @type {(props, context) => void} */(Ctor)(props, context); | ||
|
There was a problem hiding this comment. This seems strange to me. @rbiggs Can you elaborate on the thinking behind it? There was a problem hiding this comment. That's a type cast. We're casting it to a basic function to avoid type error displayed in this image:
This is a common problem when dealing with constructor functions and TypeScript. A simple type cast resolve this problem. There was a problem hiding this comment. The following change I proposed in rbiggs/preact#2 should make this read a bit better: --- a/src/vdom/component-recycler.js
+++ b/src/vdom/component-recycler.js
@@ -33,7 +33,9 @@ export function createComponent(Ctor, props, context) {
inst;
if (Ctor.prototype && Ctor.prototype.render) {
- inst = new /** @type {(props, context) => void} */(Ctor)(props, context);
+ const ComponentCtor =
+ /** @type {(props, context) => void} */(Ctor);
+ inst = new ComponentCtor (props, context);
Component.call(inst, props, context);
}
else {There was a problem hiding this comment. The variables are fine, it's the typings that I have an issue with. It seems wrong and more like an issue with typescript if one has to define a class constructor as a plain function. There was a problem hiding this comment. Actually, it's the wrong type being defined in the parameter: /**
* Create a component. Normalizes differences between PFC's and classful
* Components.
* @param {function} Ctor The constructor of the component to create
* @param {object} props The initial props of the component
* @param {object} context The initial context of the component
* @returns {import('../component').Component}
*/The Ctor type is described as a generic function, which can't be used with the /**
* Create a component. Normalizes differences between PFC's and classful
* Components.
* @param {(props, context) => void} Ctor The constructor of the component to create
* @param {object} props The initial props of the component
* @param {object} context The initial context of the component
* @returns {import('../component').Component}
*/Then the type cast isn't necessary: if (Ctor.prototype && Ctor.prototype.render) {
inst = new Ctor(props, context);
Component.call(inst, props, context);
} |
||
| Component.call(inst, props, context); | ||
| } | ||
| else { | ||
| inst = new Component(props, context); | ||
| inst.constructor = Ctor; | ||
| inst['render'] = doRender; | ||
|
There was a problem hiding this comment. Seems like an unnecessary change here There was a problem hiding this comment. The problem here is that
You can get around this in one of two ways: type casting the This is a very common problem with dealing with expando properties and type systems. TypeScript users prefer to escape them as I have done. There was a problem hiding this comment. I would reeeally like to see if we can avoid code changes for the doc annotations. This was something I had run into in another project when trying to use checkJs. Thought: is there a way for us to use a typedef to create a ficticious There was a problem hiding this comment. Should be resolved here and in other places by changes proposed in rbiggs/preact#4, according to the first recommended alternative:
I find it interesting to note his second alternative:
|
||
| } | ||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,9 +1,11 @@ | ||
| // @ts-nocheck | ||
| // Prevent warning of using VNode before declaration in defnition of children property. | ||
| /** | ||
| * Virtual DOM Node | ||
| * @typedef VNode | ||
| * @property {string | function} nodeName The string of the DOM node to create or Component constructor to render | ||
| * @property {Array<VNode | string>} children The children of node | ||
| * @property {string | number | undefined} key The key used to identify this VNode in a list | ||
| * @property {Object.<string, any>} attributes The properties of this VNode | ||
| */ | ||
| export const VNode = function VNode() {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ohh I like this one! Totally forgot about
typedef👍