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

[feat] Updated jsdoctypeparser package. #196

Merged
merged 12 commits into from Sep 7, 2022

Conversation

neogeek
Copy link
Contributor

@neogeek neogeek commented Apr 26, 2022

This PR is still very much a work in progress.

  1. Added fixture snapshots based on the codebase before the jsdoctypeparser package was updated. I can remove them before taking the PR out of draft mode.
  2. Updated the jsdoctypeparser package and usage.
  3. Removed an unsupported type test. Not sure about this change. I would still like to figure out why this type causes the parser to throw an error (below). I also updated the fixture snapshot to remove this pattern result.
    Error: SyntaxError: Expected [ \t], [\n], [\r], or end of input but "n" found.
  4. Updated some whitespace in the fixture snapshots.

The tests currently fail, but I feel like I'm close to fixing the remaining issues.

> dox@0.9.0 test
> jest test/fixtures.test.js

 FAIL  test/fixtures.test.js
  custom parser
    ✓ a (4 ms)
    ✓ alias (1 ms)
    ✓ asterik (12 ms)
    ✓ b (1 ms)
    ✓ c (8 ms)
    ✕ classes (4 ms)
    ✓ d-mixed (1 ms)
    ✓ d-spaces (1 ms)
    ✓ d-tabs (1 ms)
    ✓ d (1 ms)
    ✓ enums
    ✓ event (1 ms)
    ✓ functions (1 ms)
    ✓ issue169
    ✕ jsdoc-complex-types (6 ms)
    ✓ jshint
    ✓ literal-inline (1 ms)
    ✓ multilinetags (2 ms)
    ✓ nodescription (1 ms)
    ✓ prototypes-inline (1 ms)
    ✓ prototypes
    ✓ single-multiline (1 ms)
    ✓ single
    ✓ singleline (1 ms)
    ✓ skip_prefix
    ✓ slash
    ✓ string-quotes (1 ms)
    ✓ throws
    ✓ titles
    ✓ uncommented (1 ms)

  ● custom parser › classes

    expect(received).toEqual(expected) // deep equality

    - Expected  - 1
    + Received  + 1

    @@ -84,11 +84,11 @@
              "type": "return",
              "string": "{Overflow}",
              "types": [
                "Overflow"
              ],
    -         "typesDescription": "<a href=\"Overflow.html\">Overflow</a>",
    +         "typesDescription": "<code>Overflow</code>",
              "optional": false,
              "nullable": false,
              "nonNullable": false,
              "variable": false,
              "description": "",

      16 |   }
      17 |
    > 18 |   expect(`${JSON.stringify(results, null, 2)}\n`).toEqual(
         |                                                   ^
      19 |     readFileSync(`./test/fixtures/${filename}.json`, 'utf8')
      20 |   );
      21 | };

      at toEqual (test/fixtures.test.js:18:51)
      at Object.testFixture (test/fixtures.test.js:29:35)

  ● custom parser › jsdoc-complex-types

    expect(received).toEqual(expected) // deep equality

    - Expected  - 31
    + Received  + 25

    @@ -50,20 +50,18 @@
              "html": "<p>{number|{name:string,age:number}|Array} a</p>"
            },
            {
              "type": "returns",
              "string": "{{name:string,age:number}}",
    -         "types": [
    -           {
    +         "types": {
    -             "name": [
    +           "name": [
    -               "string"
    +             "string"
    -             ],
    +           ],
    -             "age": [
    +           "age": [
    -               "number"
    +             "number"
    -             ]
    +           ]
    -           }
    -         ],
    +         },
              "typesDescription": "{name: <code>string</code>, age: <code>number</code>}",
              "optional": false,
              "nullable": false,
              "nonNullable": false,
              "variable": false,
    @@ -102,28 +100,24 @@
                "string",
                {
                  "length": [
                    "number"
                  ],
    -             "type": [
    -               {
    +             "type": {
    -                 "name": [
    -                   {
    +               "name": {
    -                     "first": [
    +                 "first": [
    -                       "string"
    +                   "string"
    -                     ],
    +                 ],
    -                     "last": [
    +                 "last": [
    -                       "string"
    +                   "string"
    -                     ]
    +                 ]
    -                   }
    -                 ],
    -                 "id": [
    +               },
    +               "id": [
    -                   "number",
    +                 "number",
    -                   "string"
    +                 "string"
    -                 ]
    +               ]
    -               }
    +             }
    -             ]
                }
              ],
              "typesDescription": "<code>number</code> | <code>string</code> | {length: <code>number</code>, type: {name: {first: <code>string</code>, last: <code>string</code>}, id: <code>number</code> | <code>string</code>}}",
              "optional": false,
              "nullable": false,
    @@ -158,11 +152,11 @@
              "name": "a",
              "description": "",
              "types": [
                "number"
              ],
    -         "typesDescription": "<code>number</code>|<code>undefined</code>",
    +         "typesDescription": "<code>number</code>=",
              "optional": true,
              "nullable": false,
              "nonNullable": false,
              "variable": false,
              "html": "<p>{number=} a</p>"
    @@ -195,11 +189,11 @@
              "name": "a",
              "description": "",
              "types": [
                "number"
              ],
    -         "typesDescription": "<code>number</code>|<code>null</code>",
    +         "typesDescription": "?<code>number</code>",
              "optional": false,
              "nullable": true,
              "nonNullable": false,
              "variable": false,
              "html": "<p>{?number} a</p>"
    @@ -235,11 +229,11 @@
                "number"
              ],
              "typesDescription": "!<code>number</code>",
              "optional": false,
              "nullable": false,
    -         "nonNullable": true,
    +         "nonNullable": false,
              "variable": false,
              "html": "<p>{!number} a</p>"
            }
          ],
          "description": {

      16 |   }
      17 |
    > 18 |   expect(`${JSON.stringify(results, null, 2)}\n`).toEqual(
         |                                                   ^
      19 |     readFileSync(`./test/fixtures/${filename}.json`, 'utf8')
      20 |   );
      21 | };

      at toEqual (test/fixtures.test.js:18:51)
      at Object.testFixture (test/fixtures.test.js:39:11)

Test Suites: 1 failed, 1 total
Tests:       2 failed, 28 passed, 30 total
Snapshots:   0 total
Time:        0.21 s, estimated 1 s
Ran all test suites matching /test\/fixtures.test.js/i.

@Twipped
Copy link
Collaborator

Twipped commented Apr 26, 2022

Awesome! Take your time, I can't issue an update yet anyway. It turns out I don't currently have npm publish rights for the dox package, it got lost when I had them rename my account a few months back. I'm waiting to hear back from support on the matter.

@hasezoey
Copy link
Contributor

hasezoey commented Aug 5, 2022

any updates regarding this PR status of being a Draft and the npm publish rights?

@Twipped
Copy link
Collaborator

Twipped commented Aug 5, 2022

@hasezoey Publish rights have been worked out, that's how I published 0.9.1

@neogeek neogeek force-pushed the feature/updated-jsdoctypeparser branch from d6f471a to 85070b0 Compare September 2, 2022 15:15
@neogeek neogeek force-pushed the feature/updated-jsdoctypeparser branch from 85070b0 to 1c7d017 Compare September 2, 2022 16:12
@neogeek
Copy link
Contributor Author

neogeek commented Sep 2, 2022

Sorry about the severe delay in continuing work for this PR; life just got away from me.

I did make some progress in fixing the remaining issues. I also removed all of the commits related to using Jest as the test running, as I'm trying to keep this PR as simple as possible. I can add it back in another PR if you'd like.

I have one final issue that I'm bumping up against in the jsdoc-complex-types.js file:

@returns {{name:string,age:number}}

This returns the following error:

Uncaught AssertionError: expected Object { name: Array [ 'string' ], age: Array [ 'number' ] } to equal Array [ Object { name: Array [ 'string' ], age: Array [ 'number' ] } ]

I've been digging through JSDocs documentation, and I can't find any reference to returning an object like this, and from what I can grok by the syntax I would assume it would return an object rather than an array.

Again, sorry for the delay.

@Twipped
Copy link
Collaborator

Twipped commented Sep 2, 2022

@neogeek I think the issue is simply that the old jsdoctypeparser always returned record types in an array, so that's what the test checked for.

@neogeek
Copy link
Contributor Author

neogeek commented Sep 2, 2022

I figured that was the case, but I was unsure if I should just wrap the types. I pushed a commit that wraps the object in an array and all of the tests pass!

@neogeek neogeek marked this pull request as ready for review September 2, 2022 17:35
@neogeek
Copy link
Contributor Author

neogeek commented Sep 2, 2022

I did unfortunately find an error when running a custom suite of tests. It's an issue related to the types where an object will get converted to [object Object]. Looking into a fix.

@neogeek
Copy link
Contributor Author

neogeek commented Sep 6, 2022

I pushed a fix for the issue I mentioned above. I'm going to look into adding tests to cover this scenario.

@neogeek
Copy link
Contributor Author

neogeek commented Sep 6, 2022

Added a test to cover that scenario! I'll keep testing in my own environment, but I think this PR should be good to go.

@Twipped
Copy link
Collaborator

Twipped commented Sep 6, 2022

@neogeek are we calling it done now? All tests passing, are you comfortable with me merging it?

@Twipped
Copy link
Collaborator

Twipped commented Sep 6, 2022

Fuggit, yolo

@Twipped Twipped merged commit 807ed32 into tj:master Sep 7, 2022
@neogeek
Copy link
Contributor Author

neogeek commented Sep 7, 2022

I am. I would still like to create a separate PR to improve the testing suite to cover more that what it covers now, if you are ok with that. Finding something the tests do not cover made me a bit nervous.

@Twipped
Copy link
Collaborator

Twipped commented Sep 7, 2022

Please do! More tests is good

@neogeek
Copy link
Contributor Author

neogeek commented Sep 7, 2022

Will do!

@neogeek neogeek deleted the feature/updated-jsdoctypeparser branch September 7, 2022 00:02
@Twipped
Copy link
Collaborator

Twipped commented Sep 7, 2022

I'll have a new release out tonight. 1.0.0, here we come!

@Twipped
Copy link
Collaborator

Twipped commented Sep 7, 2022

We have 1.0.0! https://github.com/tj/dox/releases/tag/v1.0.0

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.

None yet

3 participants