Fix lint warnings in blockly-samples

[maribethb]

Check for duplicates

• I have searched for similar issues before opening a new one.

Component

multiple

Description

After #1946 is submitted:

There are several lint warnings in blockly-samples that we should fix or ignore the linter. Run the linter and fix the warnings shown.

Note that several of the warnings are about missing JsDoc comments, so you may need to write small amounts of documentation to fix this issue.

unexpected any warnings may need refactorings to avoid the any type, or may need to just add an ignore statement to silence the warning, if the use of any is appropriate.

Reproduction steps

1. Run npm run lint from the root of blockly-samples
2. Note the multiple lint warnings

Stack trace

No response

Screenshots

No response

[sdthaker]

Hi there! Could you please assign this to me? I'd like to work on this issue! Thanks!

[BeksOmega]

Go for it @sdthaker =) If you have any questions don't hesitate to ask!

[sdthaker]

I'd want to test the plugin field-dependent-dropdown since L150 has a warning about unexpected any usage in variable event. How can I test this plugin? I'm looking to console.log the type of the variable and assign it a proper type by running the plugin as a server or something but the readme doesn't mention anything about this. Also, is there a better way to test the variable type other than what I suggested in this comment? Thanks

[sdthaker]

I'd want to test the plugin field-dependent-dropdown since L150 has a warning about unexpected any usage in variable event. How can I test this plugin? I'm looking to console.log the type of the variable and assign it a proper type by running the plugin as a server or something but the readme doesn't mention anything about this. Also, is there a better way to test the variable type other than what I suggested in this comment? Thanks

I was looking into this warning and seems like the any type assignment is done on purpose as explained here. So I'll be adding the rule /* eslint-disable-next-line @typescript-eslint/no-explicit-any */ above L150 to silence the warning. Please let me know if I should be doing it otherwise.

[BeksOmega]

I'd want to test the plugin field-dependent-dropdown since L150 has a warning about unexpected any usage in variable event. How can I test this plugin? I'm looking to console.log the type of the variable and assign it a proper type by running the plugin as a server or something but the readme doesn't mention anything about this. Also, is there a better way to test the variable type other than what I suggested in this comment? Thanks

I was looking into this warning and seems like the any type assignment is done on purpose as explained here. So I'll be adding the rule /* eslint-disable-next-line @typescript-eslint/no-explicit-any */ above L150 to silence the warning. Please let me know if I should be doing it otherwise.

Yes that sounds perfect!

I'd want to test the plugin field-dependent-dropdown since L150 has a warning about unexpected any usage in variable event. How can I test this plugin? I'm looking to console.log the type of the variable and assign it a proper type by running the plugin as a server or something but the readme doesn't mention anything about this. Also, is there a better way to test the variable type other than what I suggested in this comment? Thanks

You can do npm run start from the field-dependent-dropdown directory to run the test page. If the plugin has tests, you can also do npm run test from the directory to run them.

[sdthaker]

One of the warnings that I'm dealing with is somewhat ambiguous since it doesn't say exactly what's the issue with the type definition. The warning is coming from L50.

Warning:

warning  Syntax error in type: {
  import: string,
  oldIdentifier?: string,
  newIdentifier: string,
  newImport: string,
  newRequire: string,
}  jsdoc/valid-types

Currently trying to fix it, any help would be appreciated! Thanks!

[BeksOmega]

One of the warnings that I'm dealing with is somewhat ambiguous since it doesn't say exactly what's the issue with the type definition. The warning is coming from L50.

Warning:

warning  Syntax error in type: {
  import: string,
  oldIdentifier?: string,
  newIdentifier: string,
  newImport: string,
  newRequire: string,
}  jsdoc/valid-types

Currently trying to fix it, any help would be appreciated! Thanks!

Hmm is that the full text of the error or is there other information? My guess is that it might be a problem with the oldIdentifier?:. Could you try changing it to oldIdentifier: string|undefined,? But I'm not sure!

[sdthaker]

One of the warnings that I'm dealing with is somewhat ambiguous since it doesn't say exactly what's the issue with the type definition. The warning is coming from L50.
Warning:

warning  Syntax error in type: {
  import: string,
  oldIdentifier?: string,
  newIdentifier: string,
  newImport: string,
  newRequire: string,
}  jsdoc/valid-types

Currently trying to fix it, any help would be appreciated! Thanks!

Hmm is that the full text of the error or is there other information? My guess is that it might be a problem with the oldIdentifier?:. Could you try changing it to oldIdentifier: string|undefined,? But I'm not sure!

Proposed code changes

Warning post adding proposed changes

TS Error post adding proposed changes

[maribethb]

My wild guess is it's the trailing comma after newRequire: string

The ? is allowed under Closure syntax, so that should be fine. You can find more info about this rule here https://github.com/gajus/eslint-plugin-jsdoc/blob/main/docs/rules/valid-types.md#readme

and valid closure types here

[sdthaker]

My wild guess is it's the trailing comma after newRequire: string

The ? is allowed under Closure syntax, so that should be fine. You can find more info about this rule here https://github.com/gajus/eslint-plugin-jsdoc/blob/main/docs/rules/valid-types.md#readme

and valid closure types here

I did try removing the trailing comma and I still received the warning! :)

[BeksOmega]

@sdthaker I think the easiest thing to do now is to put up a PR with what you've got, then I can pull it down and figure out what's up with this specific type =)

[sdthaker]

@sdthaker I think the easiest thing to do now is to put up a PR with what you've got, then I can pull it down and figure out what's up with this specific type =)

I was actually thinking of doing that today! LOL! Perfect, so I’d silence the warning through an ESLint rule for that piece if code and we can take it up later if the code changes needs to be removed based on your feedback!

[maribethb]

You can also just leave it un-silenced. Warnings don't block the CI or anything, and this is one we should probably actually resolve if there's really an invalid type, so it's fine to not be silenced. Thanks for looking into this, Beka!