Skip to content

Non-optional fields causing TypeScript errors #749

Closed

Description

@paralin

protobuf.js version: 6.7.1

I'm passing the output of pbjs json conversion to loadJson, this works fine, but there's a new typing error in the latest update:

Argument of type '{ "nested": { "google": { "nested": { "api": { "options": { "java_multiple_files": boolean; "java...' is not assignable to parameter of type 'NamespaceDescriptor'.
  Types of property 'nested' are incompatible.
    Type '{ "google": { "nested": { "api": { "options": { "java_multiple_files": boolean; "java_outer_class...' is not assignable to type '{ [k: string]: EnumDescriptor | ExtensionFieldDescriptor | ExtensionMapFieldDescriptor | TypeDesc...'.
      Property '"google"' is incompatible with index signature.
        Type '{ "nested": { "api": { "options": { "java_multiple_files": boolean; "java_outer_classname": strin...' is not assignable to type 'EnumDescriptor | ExtensionFieldDescriptor | ExtensionMapFieldDescriptor | TypeDescriptor | Servic...'.
          Type '{ "nested": { "api": { "options": { "java_multiple_files": boolean; "java_outer_classname": strin...' is not assignable to type 'ServiceDescriptor'.
            Property 'methods' is missing in type '{ "nested": { "api": { "options": { "java_multiple_files": boolean; "java_outer_classname": strin...'

The issue is that the "methods" field is not optional. Could you please set that field to optional in the typings?

Thanks!

Activity

  1. dcodeIO commented on Apr 5, 2017

    @dcodeIO
    Member

    methods is actually required, it marks an object to be a service. Without it, fromJSON doesn't know what kind of object it is handling.

    There is a table of required properties of the different JSON structures here.

    If you generated the respective JSON descriptor somehow, there might be something wrong in generation instead.

    Alternatively, the issue could be more general in that the object does not match any valid type. For instance, if google is just a namespace, it should resemble an empty message (an object with a fields = {} property) within a JSON descriptor.

  2. paralin commented on Apr 5, 2017

    @paralin
    Author

    Right, but it's not required on every single thing, right? You can treat it not being there as null. If I just ignore the typing error protobufjs works correctly anyway right now.

  3. dcodeIO commented on Apr 5, 2017

    @dcodeIO
    Member

    Yeah, without a marker, addJSON falls back to treat the object as a namespace. Inner namespaces aren't really intended, though, but could be added somewhere around here. Issue here is that nested is optional on NamespaceDescriptor because it's abstract, effectively making the type accept empty objects.

  4. paralin commented on Apr 5, 2017

    @paralin
    Author

    I nest namespaces in my code, quite a bit actually. It seems to work just fine! Is this just a typings issue, or does something need to be changed in ProtobufJS to support that pattern? Assuming the fallback is actually working as intended and I'm not abusing some kind of bug.

  5. dcodeIO commented on Apr 5, 2017

    @dcodeIO
    Member

    More of a typings issue because of empty objects.

    Issue here is that nested is optional on NamespaceDescriptor because it's abstract, effectively making the type accept empty objects.

  6. dcodeIO commented on Apr 5, 2017

    @dcodeIO
    Member

    Currently trying to fix this, but as soon as I add a sixth type to

    @type {EnumDescriptor|TypeDescriptor|ServiceDescriptor|ExtensionFieldDescriptor|ExtensionMapFieldDescriptor}

    my VSCode hangs with 100% cpu. That's new. Lots of recursion if I had to guess.

  7. paralin commented on Apr 5, 2017

    @paralin
    Author

    My tslint did the same thing for the longest time, that's probably your issue. I think it has some unchecked infinite recursion if you use recursive structures in your code.

    The issue is with only one of the checkers - the "unused variable" checker. But you're not even using tslint, I think. Your vscode might be using it internally. You could try turning off that checker in a little tslint.json file and see if it fixes it.

  8. dcodeIO commented on Apr 5, 2017

    @dcodeIO
    Member

    Actually had a tslint plugin installed but the issue remains after uninstalling it. That's amazing...

  9. paralin commented on Apr 5, 2017

    @paralin
    Author

    eslint? Wait, no, you had to be using tslint, if the issue started happening after you changed the typings.

    I would try dropping a tslint.json in the root of your repo, here's mine:

    [removed]

    There are some Angular2 rules in there that you'll get errors about but you can just delete those lines. The most important part is to turn off the unused variable analysis.

  10. paralin commented on Apr 5, 2017

    @paralin
    Author

    Here's the non-angular2 one with the unused variable checker disabled:

    "no-unused-expression": false,

    {
      "rules": {
        "class-name": true,
        "comment-format": [
          true,
          "check-space"
        ],
        "curly": true,
        "eofline": true,
        "forin": true,
        "indent": [
          true,
          "spaces"
        ],
        "label-position": true,
        "max-line-length": [
          true,
          140
        ],
        "member-access": false,
        "member-ordering": [
          true,
          "static-before-instance",
          "variables-before-functions"
        ],
        "no-arg": true,
        "no-bitwise": true,
        "no-console": [
          true,
          "debug",
          "info",
          "time",
          "timeEnd",
          "trace"
        ],
        "no-construct": true,
        "no-debugger": true,
        "no-duplicate-variable": true,
        "no-empty": false,
        "no-eval": true,
        "no-inferrable-types": true,
        "no-shadowed-variable": true,
        "no-string-literal": false,
        "no-switch-case-fall-through": true,
        "no-trailing-whitespace": true,
        "no-unused-expression": false,
        "no-use-before-declare": false,
        "no-var-keyword": true,
        "object-literal-sort-keys": false,
        "one-line": [
          true,
          "check-open-brace",
          "check-catch",
          "check-else",
          "check-whitespace"
        ],
        "quotemark": [
          true,
          "single"
        ],
        "radix": true,
        "semicolon": [
          "always"
        ],
        "triple-equals": [
          true,
          "allow-null-check"
        ],
        "typedef-whitespace": [
          true,
          {
            "call-signature": "nospace",
            "index-signature": "nospace",
            "parameter": "nospace",
            "property-declaration": "nospace",
            "variable-declaration": "nospace"
          }
        ],
        "variable-name": false,
        "whitespace": [
          true,
          "check-branch",
          "check-decl",
          "check-operator",
          "check-separator",
          "check-type"
        ]
      }
    }
    
  11. dcodeIO commented on Apr 5, 2017

    @dcodeIO
    Member
  12. dcodeIO commented on Apr 5, 2017

    @dcodeIO
    Member

    Well, I introduced a new otherwise useless type AnyExtensionFieldDescriptor covering two types making it a total of 5 again in AnyNestedDescriptor. Problem solved.

    vscode issue: microsoft/vscode#23958

  13. dcodeIO commented on Apr 11, 2017

    @dcodeIO
    Member

    Should be working with 6.7.3. Feel free to reopen if it's not!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions