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

Change structs for generic types #3442

Closed
wants to merge 3 commits into from
Closed

Change structs for generic types #3442

wants to merge 3 commits into from

Conversation

orblazer
Copy link
Contributor

@orblazer orblazer commented Mar 21, 2018

This PR changes (delete as applicable)

  • Documentation

Describe the changes below:

I have change the structs type for make generics types like Array and Object types.

The types integer and float is wrong or is correct ? If is correct we need create an alias for number type.

@photonstorm
Copy link
Collaborator

Where are the templates for T and K defined?

integer and float are fine and define an explicit fixed type, where number could be either.

@photonstorm
Copy link
Collaborator

Just to add, why would we need an alias for number? These are all valid numeric types.

@orblazer
Copy link
Contributor Author

The template is delcrared here Map.js#28 and is it same on all structs files.

Yes in programing the type integer and float is rigth but in JS we have only number type.

@photonstorm
Copy link
Collaborator

photonstorm commented Mar 21, 2018

Are you sure @template is actually parsed by jsdoc and emits a valid type in the output? It's not listed as a known tag on their site.

The purpose of integer and float are to explain intent to someone reading the docs, regardless if it's a valid data type in JS. If TS doesn't support them then they'll need converting to number types as part of the conversion process (but not changed in the jsdocs themselves as they're valid output for the doc generator)

@orblazer
Copy link
Contributor Author

Hum I don't find references for integer or float in jsdocs.

The @template is an tag from Google Closure but jsdocs supports that.

Src: jsdoc/jsdoc#605

@photonstorm
Copy link
Collaborator

It doesn't matter that int/f aren't listed in jsdocs, neither is the @webglOnly tag I use a lot, what matters is that the docs template knows about them and uses them, which helps the devs reading them. For the sake of conversion to TS they'll need converting to number types.

I need to run some tests to see what the jsdoc output looks like when it hits a template definition, because when browsing the source code it is now completely meaningless, but as long as jsdoc understands the type is actually an object and not just the letter T then it will be fine.

@orblazer
Copy link
Contributor Author

OK I can write an test tomorrow (I'm French) and for int/float we can make an alias for the ide recognize that types.

@photonstorm
Copy link
Collaborator

Ok, as it stands currently jsdoc is having trouble parsing some of the new types. Here is a full dump of all the errors (this is from a branch created from this PR):

ERROR: Unable to parse a tag's type expression for source file D:\wamp\www\phaser\src\display\color\Color.js in line 80 with tag title "type" and text "{[number,number,number,number]}": Invalid type expression "[number,number,number,number]": Expected "!", "$", "'", "(", "*", ".", "...", "0", "?", "@", "Function", "\"", "\\", "_", "break", "case", "catch", "class", "const", "continue", "debugger", "default", "delete", "do", "else", "enum", "export", "extends", "false", "finally", "for", "function", "if", "implements", "import", "in", "instanceof", "interface", "let", "new", "null", "package", "private", "protected", "public", "return", "static", "super", "switch", "this", "throw", "true", "try", "typeof", "undefined", "var", "void", "while", "with", "yield", "{", Unicode letter number, Unicode lowercase letter, Unicode modifier letter, Unicode other letter, Unicode titlecase letter, Unicode uppercase letter or
[1-9] but "[" found.
ERROR: Unable to parse a tag's type expression for source file D:\wamp\www\phaser\src\display\color\Color.js in line 87 with tag title "type" and text "{[number,number,number,number]}": Invalid type expression "[number,number,number,number]": Expected "!", "$", "'", "(", "*", ".", "...", "0", "?", "@", "Function", "\"", "\\", "_", "break", "case", "catch", "class", "const", "continue", "debugger", "default", "delete", "do", "else", "enum", "export", "extends", "false", "finally", "for", "function", "if", "implements", "import", "in", "instanceof", "interface", "let", "new", "null", "package", "private", "protected", "public", "return", "static", "super", "switch", "this", "throw", "true", "try", "typeof", "undefined", "var", "void", "while", "with", "yield", "{", Unicode letter number, Unicode lowercase letter, Unicode modifier letter, Unicode other letter, Unicode titlecase letter, Unicode uppercase letter or
[1-9] but "[" found.
WARNING: The @type tag does not permit a description; the description will be ignored. File: LoaderPlugin.js, line: 201
WARNING: The @type tag does not permit a description; the description will be ignored. File: LoaderPlugin.js, line: 208
ERROR: Unable to parse a tag's type expression for source file D:\wamp\www\phaser\src\math\random-data-generator\RandomDataGenerator.js in line 172 with tag title "param" and text "{*[]} seeds - The array of seeds: the `toString()` of each value is used.": Invalid type expression "*[]": Expected "!", "=", "?", "|" or end of input but "[" found.
ERROR: Unable to parse a tag's type expression for source file D:\wamp\www\phaser\src\math\random-data-generator\RandomDataGenerator.js in line 182 with tag title "param" and text "{*[]} seeds - The array of seeds: the `toString()` of each value is used.": Invalid type expression "*[]": Expected "!", "=", "?", "|" or end of input but "[" found.
ERROR: Unable to parse a tag's type expression for source file D:\wamp\www\phaser\src\physics\arcade\World.js in line 1786 with tag
title "param" and text "{*[]} arr": Invalid type expression "*[]": Expected "!", "=", "?", "|" or end of input but "[" found.
ERROR: Unable to parse a tag's type expression for source file D:\wamp\www\phaser\src\physics\arcade\World.js in line 1795 with tag
title "param" and text "{*[]} arr": Invalid type expression "*[]": Expected "!", "=", "?", "|" or end of input but "[" found.
ERROR: The @property tag requires a value. File: Scene.js, line: 10
ERROR: Unable to parse a tag's type expression for source file D:\wamp\www\phaser\src\scene\Settings.js in line 11 with tag title "property" and text "{(false|[type])} [files=false] - [description]": Invalid type expression "(false|[type])": Expected "!", "$", "'", "(", "*", ".", "...", "0", "?", "@", "Function", "\"", "\\", "_", "break", "case", "catch", "class", "const", "continue", "debugger", "default", "delete", "do", "else", "enum", "export", "extends", "false", "finally", "for", "function", "if", "implements", "import", "in", "instanceof", "interface", "let", "new", "null", "package", "private", "protected", "public", "return", "static", "super", "switch", "this", "throw", "true", "try", "typeof", "undefined", "var", "void", "while", "with", "yield", "{", Unicode letter number, Unicode lowercase letter, Unicode modifier letter, Unicode other letter, Unicode titlecase letter, Unicode uppercase letter or [1-9] but "[" found.
ERROR: Unable to parse a tag's type expression for source file D:\wamp\www\phaser\src\scene\Settings.js in line 11 with tag title "property" and text "{(false|[type])} [plugins=false] - [description]": Invalid type expression "(false|[type])": Expected "!", "$", "'", "(", "*", ".", "...", "0", "?", "@", "Function", "\"", "\\", "_", "break", "case", "catch", "class", "const", "continue", "debugger", "default", "delete", "do", "else", "enum", "export", "extends", "false", "finally", "for", "function", "if", "implements", "import", "in", "instanceof", "interface", "let", "new", "null", "package", "private", "protected", "public", "return", "static", "super", "switch", "this", "throw", "true", "try", "typeof", "undefined", "var", "void", "while", "with", "yield", "{", Unicode letter number, Unicode lowercase letter, Unicode modifier letter, Unicode other letter, Unicode titlecase letter, Unicode uppercase letter or [1-9] but "[" found.
ERROR: Unable to parse a tag's type expression for source file D:\wamp\www\phaser\src\scene\Settings.js in line 25 with tag title "property" and text "{(false|[type])} files - [description]": Invalid type expression "(false|[type])": Expected "!", "$", "'", "(", "*", ".", "...", "0", "?", "@", "Function", "\"", "\\", "_", "break", "case", "catch", "class", "const", "continue", "debugger", "default", "delete", "do", "else", "enum", "export", "extends", "false", "finally", "for", "function", "if", "implements", "import", "in", "instanceof", "interface", "let", "new", "null", "package", "private", "protected", "public", "return", "static", "super", "switch", "this", "throw", "true", "try", "typeof", "undefined", "var", "void", "while", "with", "yield", "{", Unicode letter number, Unicode lowercase letter, Unicode modifier letter, Unicode other letter, Unicode titlecase letter, Unicode uppercase letter or [1-9] but "[" found.
ERROR: Unable to parse a tag's type expression for source file D:\wamp\www\phaser\src\scene\Settings.js in line 25 with tag title "property" and text "{(false|[type])} plugins - [description]": Invalid type expression "(false|[type])": Expected "!", "$", "'", "(", "*", ".", "...", "0", "?", "@", "Function", "\"", "\\", "_", "break", "case", "catch", "class", "const", "continue", "debugger", "default", "delete", "do", "else", "enum", "export", "extends", "false", "finally", "for", "function", "if", "implements", "import", "in", "instanceof", "interface", "let", "new", "null", "package", "private", "protected", "public", "return", "static", "super", "switch", "this", "throw", "true", "try", "typeof", "undefined", "var", "void", "while", "with", "yield", "{", Unicode letter number, Unicode lowercase letter, Unicode modifier letter, Unicode other letter, Unicode titlecase letter, Unicode uppercase letter or [1-9] but "[" found.
ERROR: Unable to parse a tag's type expression for source file D:\wamp\www\phaser\src\scene\Settings.js in line 42 with tag title "property" and text "{(false|[type])} files - [description]": Invalid type expression "(false|[type])": Expected "!", "$", "'", "(", "*", ".", "...", "0", "?", "@", "Function", "\"", "\\", "_", "break", "case", "catch", "class", "const", "continue", "debugger", "default", "delete", "do", "else", "enum", "export", "extends", "false", "finally", "for", "function", "if", "implements", "import", "in", "instanceof", "interface", "let", "new", "null", "package", "private", "protected", "public", "return", "static", "super", "switch", "this", "throw", "true", "try", "typeof", "undefined", "var", "void", "while", "with", "yield", "{", Unicode letter number, Unicode lowercase letter, Unicode modifier letter, Unicode other letter, Unicode titlecase letter, Unicode uppercase letter or [1-9] but "[" found.
ERROR: Unable to parse a tag's type expression for source file D:\wamp\www\phaser\src\scene\Settings.js in line 42 with tag title "property" and text "{(false|[type])} plugins - [description]": Invalid type expression "(false|[type])": Expected "!", "$", "'", "(", "*", ".", "...", "0", "?", "@", "Function", "\"", "\\", "_", "break", "case", "catch", "class", "const", "continue", "debugger", "default", "delete", "do", "else", "enum", "export", "extends", "false", "finally", "for", "function", "if", "implements", "import", "in", "instanceof", "interface", "let", "new", "null", "package", "private", "protected", "public", "return", "static", "super", "switch", "this", "throw", "true", "try", "typeof", "undefined", "var", "void", "while", "with", "yield", "{", Unicode letter number, Unicode lowercase letter, Unicode modifier letter, Unicode other letter, Unicode titlecase letter, Unicode uppercase letter or [1-9] but "[" found.
ERROR: Unable to parse a tag's type expression for source file D:\wamp\www\phaser\src\structs\Map.js in line 19 with tag title "param" and text "{*[]} elements - [description]": Invalid type expression "*[]": Expected "!", "=", "?", "|" or end of input but "[" found.
ERROR: Unable to parse a tag's type expression for source file D:\wamp\www\phaser\src\structs\Map.js in line 37 with tag title "param" and text "{*[]} elements - [description]": Invalid type expression "*[]": Expected "!", "=", "?", "|" or end of input but "[" found.
ERROR: Unable to parse a tag's type expression for source file D:\wamp\www\phaser\src\time\Clock.js in line 152 with tag title "param" and text "{*[]} args - [description]": Invalid type expression "*[]": Expected "!", "=", "?", "|" or end of input but "[" found.
ERROR: Unable to parse a tag's type expression for source file D:\wamp\www\phaser\src\time\Clock.js in line 165 with tag title "param" and text "{*[]} args - [description]": Invalid type expression "*[]": Expected "!", "=", "?", "|" or end of input but "[" found.
ERROR: Unable to parse a tag's type expression for source file D:\wamp\www\phaser\src\time\TimerEvent.js in line 10 with tag title "property" and text "{*[]} [args] - [description]": Invalid type expression "*[]": Expected "!", "=", "?", "|" or end of input but "[" found.

I checked one of the new @template types and it doesn't understand what they are. It allows them through because the config setting is lenient, but this is what they come out as in the doclets:

    {
      "comment": "/**\r\n     * [description]\r\n     *\r\n     * @method Phaser.Structs.Map#set\r\n     * @since 3.0.0\r\n     *\r\n     * @param {object} key - [description]\r\n     * @param {T} value - [description]\r\n     *\r\n     * @return {Phaser.Structs.Map} This Map object.\r\n     */",
      "meta": {
        "filename": "Map.js",
        "lineno": 72,
        "columnno": 4,
        "path": "D:\\wamp\\www\\phaser\\src\\structs",
        "code": {}
      },
      "description": "[description]",
      "kind": "function",
      "name": "set",
      "since": "3.0.0",
      "params": [
        {
          "type": {
            "names": [
              "K"
            ]
          },
          "description": "[description]",
          "name": "key"
        },
        {
          "type": {
            "names": [
              "T"
            ]
          },
          "description": "[description]",
          "name": "value"
        }
      ],
      "returns": [
        {
          "type": {
            "names": [
              "Phaser.Structs.Map"
            ]
          },
          "description": "This Map object."
        }
      ],
      "memberof": "Phaser.Structs.Map",
      "longname": "Phaser.Structs.Map#set",
      "scope": "instance",
      "___id": "T000002R024355",
      "___s": true
    },

It doesn't appear to have a clue what the type is, so has just inserted it 'T' and 'K', which is a shame - I thought the point of a template was that it would map it back to an object again, like a TypeDef does.

Not really sure what to do about this, as it stands it will literally print the types of the Map.set method as T and K in the output, which is a bit nuts :)

@orblazer
Copy link
Contributor Author

This is logic out T and K type because is it generic type. But if you want we can rename Generic names Like T > VALUE and K > KEY

I have try search if we can make predefined generic like my exemple (<G extends Phaser.GameObjects.GameObject>) but i don't have found any solution for that :(

The generic type is pretty usefull for make TypeDefs. I search an method for make generic types for Actions methods for increase autocomplete when use Typescript.

@orblazer
Copy link
Contributor Author

Sorry for this commit : 63c9938
But i can't remove that commit ;(

@photonstorm
Copy link
Collaborator

Could you use a custom tag? (something like @generic T) which the converter could then read and change the type accordingly? Then it will be useful in the TS Defs without making the JSDoc unreadable.

I just released 3.3, so I can close off this PR if you like and you could rebase against 3.4?

@orblazer
Copy link
Contributor Author

OK, seen that i rewrite all code we can close this PR.

@orblazer orblazer deleted the genericStruct branch March 22, 2018 14:12
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.

2 participants