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

Way to feed no-undefined-types additional files or have it auto-detect them (and finding unused types within the same set of files); avoid defects of tampering with no-unused-vars #99

Closed
brettz9 opened this issue Oct 31, 2018 · 27 comments

Comments

@brettz9
Copy link
Collaborator

brettz9 commented Oct 31, 2018

For no-undefined-types, I'd like a way to feed one's project files, whether:

  1. Detected by static or dynamic import/require, with the entry file(s) determined by:
    1. a new eslint-plugin-jsdoc option
    2. use of main or exports in package.json
    3. use of jsdoc config
  2. By a file being present in one's jsdoc config
  3. If eslint-plugin-jsdoc were supplied an array of files for finding typedefs

I need more context because some of my types are in:

  • A purely-comment-based typedefs.js file which includes external and generic types
  • Other files and modules

I'd like it to report for:

/**
* @param {MyType}
*/

...where MyType is not defined anywhere within JSDoc typedefs/interfaces across all files. The corollary is thus not to report errors for:

// One file, such as `typedefs.js`
/**
* @typedef {object} MyType
*/
/**
* @interface AnotherType
*/

// Another file:
/**
* @param {MyType}
*/

/**
* @param {AnotherType}
*/

Thanks very much for the very useful plugin!

Update: While traversing files of a project, also consider supporting @inheritdoc per #765


Want to back this issue? Post a bounty on it! We accept bounties via Bountysource.

@gajus
Copy link
Owner

gajus commented Oct 31, 2018

@gajus gajus closed this as completed Oct 31, 2018
@brettz9
Copy link
Collaborator Author

brettz9 commented Oct 31, 2018

Given that JSDoc has its own means of expressing user custom types such as through @typedef and @interface, wouldn't JSDoc types make sense as the default way to feed in definitions (and without extra effort to redefine such types)?

@brettz9
Copy link
Collaborator Author

brettz9 commented Oct 31, 2018

Btw, re: those PRs, I am quite impressed how intuitive your code is, that it looks very easy to make one... Will see if I can set aside some time for it...

@gajus
Copy link
Owner

gajus commented Oct 31, 2018

Given that JSDoc has its own means of expressing user custom types such as through @typedef and @interface, wouldn't JSDoc types make sense as the default way to feed in definitions (and without extra effort to redefine such types)?

Misunderstood the original issue.

@gajus gajus reopened this Oct 31, 2018
@brettz9 brettz9 changed the title Way to feed no-undefined-types additional files Way to feed no-undefined-types additional files (and finding unused types with the same data) Nov 7, 2018
@brettz9 brettz9 changed the title Way to feed no-undefined-types additional files (and finding unused types with the same data) Way to feed no-undefined-types additional files (and finding unused types within the same set of iles) Nov 7, 2018
@bary12
Copy link
Contributor

bary12 commented Dec 3, 2018

Generally, The ESLint-friendly way of doing this is exporting the types using /* exported MyType */ and importing them as /* global MyType */, this is also how ESLint suggests to deal with variables defined in different files in browser context (without modules) when using no-unused-vars and no-undef. I don't think there's a general need in the community for this feature, and exported/global can be satisfactory.

@brettz9
Copy link
Collaborator Author

brettz9 commented Dec 3, 2018

@bary12 : Maybe you have misunderstood the request also? This is not talking about JavaScript variables representing types, but JSDoc types... I've updated my original post with some examples...

If you did actually mean leveraging exported/imported for JSDoc types, besides the fact that the no-unused-vars rule would be reported when the type was not used in code, it would be awfully cumbersome to import and export JSDoc types manually when the information was easily accessible elsewhere within JSDoc comments.

@bary12
Copy link
Contributor

bary12 commented Dec 4, 2018

The rule treats JSDoc types defined in typedefs as normal variables. It scans the file for typedefs and adds them to the scope exactly like normal variables.

@brettz9
Copy link
Collaborator Author

brettz9 commented Dec 4, 2018

Yes, but it only scans the current file... Note my reference in the updated comment to these being in different files.. If one has defined generic types like "PlainObject" in another file, one would have to copy the definitions back in to each file, which is very cumbersome...

@bary12
Copy link
Contributor

bary12 commented Dec 4, 2018

Or use /* global PlainObject */, the same way as ESLint recommends doing for variables when you have no form of import/export, or even add PlainObject to the list of globals in eslintrc.

@brettz9
Copy link
Collaborator Author

brettz9 commented Dec 4, 2018

Gotcha... Yes, that is better than pasting in the whole typedef and the list of globals in .eslintrc would at least work across files, but besides adding redundant work--adding comments to lint one's comments seems particularly wasteful--I don't think type information should be polluting variable information.

For one, it may mislead developers into thinking that such a variable is in use within the file. Secondly, this may distract from what the real globals are, with globals often being a temporary evil necessity that should be easier to quickly review and occasionally re-review for possible removal/conversion to modules; if types are mixed in there, it becomes harder to notice the real global variables in use. Thirdly, it may improperly allow variables as types where the variables of that name don't actually exist.

@brettz9 brettz9 changed the title Way to feed no-undefined-types additional files (and finding unused types within the same set of iles) Way to feed no-undefined-types additional files (and finding unused types within the same set of files) Apr 4, 2019
@brettz9
Copy link
Collaborator Author

brettz9 commented Apr 8, 2019

If jsdoc/jsdoc#1632 gets approved (for jsdoc to support Typescript documentation's import syntax), this issue may be easier to resolve, as files would point to the modules from which they were "importing" documentation.

@lll000111
Copy link

lll000111 commented May 15, 2019

@brettz9 Not if the @typedef was made with @global. The reason for @global in my case is that only then does jsdoc create links to the type (in the HTML it generates). I prefer sending all my readers to some "global" file with all types even if it gets big and has them all mixed over expecting them to scroll to the bottom of whatever module file HTML doc they are in to find the type — most people will just think there is no documentation for the type if it's not a link.

@brettz9
Copy link
Collaborator Author

brettz9 commented May 15, 2019

AFAICT, we are introspecting on all typedefs with a single file. If we allow input from additional files, the files with the @global @typedef's can be fed in too. As far as making the links work, one has to do some serious finagling (and possibly tag abusing) to get them to work in some instances. You might take a look at our https://github.com/SVG-Edit/svgedit code (with docs at https://svg-edit.github.io/svgedit/releases/latest/docs/jsdoc/module-SVGEditor.html ) which managed to get modules linking together. I think in a few cases, I had to just make a link rather than inlining other content, but I don't think there are any pointers to dead links. But even in this project, we have a global typedefs file for defining types like "Float", so I'd rather not that each file had to redefine that type.

@lll000111
Copy link

so I'd rather not that each file had to redefine that type.

Okay, so I'm a bit confused, I was talking about jsdoc the HTML creating tool so no complaint about this plugin. It's their bug or missing feature, but given the lack of development there I didn't file an issue but just used whatever workaround(s) I need (which are a lot). Linking within the same generated HTML file should be just as easy as to the global file for jsdoc. In the end you seem to agree that types defined with @global not in the same file are a quite normal use case, so they should not cause an "is undefined" warning, even if that takes quite a bit more effort (and since local information such as the import statements are of no help since they won't ... ahem... okay forget what I wrote, I will still have my TypeScript of Flow import statements so yes, you could use those, as you said. I just got up and the brain isn't fully working yet....

@lll000111
Copy link

Actually, now I remember what I was thinking: Using import statements only works for types used directly for type annotations. Types mentioned/linked in text might not be imported by the type checker and would be a JSDoc-only issue.

@brettz9
Copy link
Collaborator Author

brettz9 commented May 15, 2019

Good point; but we could still add that as part of the scan if we were following those links anyways.

@brettz9
Copy link
Collaborator Author

brettz9 commented Nov 8, 2019

One approach we can use here for regular jsdoc is to parse (and cache) a jsdoc config file: https://jsdoc.app/about-configuring-jsdoc.html (or a string containing the equivalent of jsdoc command line args).

(We could also use this config file to feed info on allowed tags in place of the definedTags option for check-tag-names.)

@brettz9
Copy link
Collaborator Author

brettz9 commented Dec 21, 2019

Besides for @typedef I've come across a few other cases where it would be useful to intelligently parse jsdoc tags across multiple files, i.e., for @todo and @license (and it may be useful just for getting at other tags, e.g., finding an API which accepts or returns a certain type). I'm thinking that as a precursor to this issue, a separate repo just for iterating files and collecting desired jsdoc metadata (whether specifying contexts, e.g., functions, or not) would be in order (including the option to follow the files via static or dynamic imports/require).

My comment here is therefore just a suggestion that implementing this be done as a generic tool so it can be reused with other projects (e.g., perhaps by license-checker or the todo tool concept outlined in #299 ) as well as for @typedef reading.

@brettz9
Copy link
Collaborator Author

brettz9 commented Dec 21, 2019

And actually, I think we can just extract much of what we have already (that is not ESLint-specific, like settings retrieval) out of the iterateJsdoc.js file.

@brettz9 brettz9 changed the title Way to feed no-undefined-types additional files (and finding unused types within the same set of files) Way to feed no-undefined-types additional files or have it auto-detect them (and finding unused types within the same set of files) Mar 24, 2020
@brettz9
Copy link
Collaborator Author

brettz9 commented May 8, 2020

I've created https://github.com/brettz9/es-file-traverse which should hopefully help with this issue. It allows iterating through JavaScript files (including cyclic dependencies), but only the files found to be in use (through import, require, or for AMD, define) given some entry file(s) (HTML or JavaScript), and supports supplying a callback which is passed the site source and AST which could be used to find the typedefs in use in a project. I imagine we could accept user options in eslint-plugin-jsdoc for the user's project's entry file(s) (or just use package.json main) and use es-file-traverse with these files so as to find all the typedefs in use in the project. (It'd still be good though to be able to look at the jsdoc config source property, especially since es-file-traverse can also handle globs, albeit not in the same exact manner.)

Too busy with other plans to implement myself these days, but I could see about helping if someone else wanted to give it a shot.

@brettz9
Copy link
Collaborator Author

brettz9 commented Sep 29, 2020

Unfortunately, ESLint does not currently support async rules. I'm not sure whether we might be able to avoid reporting on the first run, waiting for file retrieval results (which we would then cache), and then somehow retrigger reporting or report on any subsequent runs. This may at least make the rule useful within IDEs.

es-file-traverse could be refactored to allow work synchronously, but I'm not really inclined to go that route.

@es50678

This comment was marked as off-topic.

@brettz9 brettz9 changed the title Way to feed no-undefined-types additional files or have it auto-detect them (and finding unused types within the same set of files); avoid defects of tampering with no-used-vars Way to feed no-undefined-types additional files or have it auto-detect them (and finding unused types within the same set of files); avoid defects of tampering with no-unused-vars Mar 2, 2022
@JoernBerkefeld
Copy link

seeing how this ticket exists... I was just trying to clean up my project when I came across this working tutorial: https://stackoverflow.com/a/55767692/818732

basically it says one should extend jsconfig.json to also read whatever js files one uses to store @typedefs in to enable intellisense to do its magic in vscode.
did that, got really happy that Intellisense did in fact work this way. Then noticed how eslint started to throw errors /warnings about it.

Couldnt we follow a similar approach here? a simple "look here to find my type defs" config variable that is parsed on top of currently open files?

@brettz9
Copy link
Collaborator Author

brettz9 commented May 28, 2023

With #1098 now dropping jsdoc/no-undefined-types from the recommended rules for typescript configs (including a new config recommended-typescript-flavor for plain JavaScript), and with TypeScript handling this properly, I think we can drop this. We can accept PRs if anyone can fix this for non-TypeScript modes, but I don't think this should be a priority with TypeScript now being the default mode.

@brettz9 brettz9 closed this as completed May 28, 2023
@what1s1ove
Copy link

what1s1ove commented Dec 29, 2023

With #1098 now dropping jsdoc/no-undefined-types from the recommended rules for typescript configs (including a new config recommended-typescript-flavor for plain JavaScript), and with TypeScript handling this properly, I think we can drop this. We can accept PRs if anyone can fix this for non-TypeScript modes, but I don't think this should be a priority with TypeScript now being the default mode.

Hey @brettz9 !

I believe there is still a problem here. With more complex types, such as when generics are present in the function, we will still encounter an error stating that the variable is not being used:

❌ Linter error: @typescript-eslint/no-unused-vars / no-unused-vars: HTMLFormOperationalControlElement in unused

/**
 * @param {<T extends unknown>(element: HTMLFormOperationalControlElement) => T} getFormControlElementPayloadCallback
 * @returns {void}
 */
const getFieldsetControlElementValue = (

✅ There is no any linter errors:

/**
 * @param {(element: HTMLFormOperationalControlElement) => unknown} getFormControlElementPayloadCallback
 * @returns {void}
 */
const getFieldsetControlElementValue = (

@brettz9
Copy link
Collaborator Author

brettz9 commented Dec 29, 2023

I'll happily accept PRs, but digging into scope analysis, assuming it can be done properly given the bugs we've seen with this rule previously, is not something I have the time or energy to tackle myself.

mbland added a commit to mbland/rollup-plugin-handlebars-precompiler that referenced this issue Jan 10, 2024
The code is still JavaScript, but now we get strict type checking in
Visual Studio Code and in continuous integration via `tsc` in `pnpm
typecheck`.

The docs generated by 'jsdoc' are a little funky, and we don't get as
much documentation in Visual Studio Code as I expected. I believe I can
fix these issues at some point with this foundation in place.

The actual changes include:

- Added @types/{chai,node}, jsdoc, and typescript as devDependencies.

- Added JSDoc-based @typedefs, including the standalone lib/types.js
  based on: "Stack Overflow: How to 'import' a typedef from one file to
  another in JSDoc using Node.js?"

  - https://stackoverflow.com/a/76872194

- Set .eslintrc to disable the no-undefined-types rule by extending
  "plugin:jsdoc/recommended-typescript-flavor-error". This is because
  the Handlebars types in lib/parser.js weren't trivial to replicate,
  and TypeScript finds those types just fine. This was based on advice
  from:

  > ...the config plugin:jsdoc/recommended-typescript-error should
  > disable the jsdoc/no-undefined-types rule because TypeScript itself
  > is responsible for reporting errors about invalid JSDoc types.
  >
  > - gajus/eslint-plugin-jsdoc#888 (comment)

  And:

  > If you are not using TypeScript syntax (your source files are still
  > .js files) but you are using the TypeScript flavor within JSDoc
  > (i.e., the default "typescript" mode in eslint-plugin-jsdoc) and you
  > are perhaps using allowJs and checkJs options of TypeScript's
  > tsconfig.json), you may use:
  >
  > ```json
  > {
  >   "extends": ["plugin:jsdoc/recommended-typescript-flavor"]
  > }
  > ```
  >
  > ...or to report with failing errors instead of mere warnings:
  >
  > ```json
  > {
  >   "extends": ["plugin:jsdoc/recommended-typescript-flavor-error"]
  > }
  > ```
  >
  > - https://github.com/gajus/eslint-plugin-jsdoc#eslintrc

  More background:

  - https://github.com/gajus/eslint-plugin-jsdoc/blob/main/docs/rules/no-undefined-types.md
  - gajus/eslint-plugin-jsdoc#99
  - gajus/eslint-plugin-jsdoc#1098
  - jsdoc/jsdoc#1537

- At the same time, extending "recommended-typescript-flavor-error"
  required adding the `// eslint-disable-next-line no-unused-vars`
  directive before each set of imports from lib/types.js.

- Added test/vitest.d.ts so TypeScript could find the custom toStartWith
  and toEndWith expect extension matchers.

- Added `pnpm typecheck && pnpm jsdoc` to `pnpm test:ci`.
mbland added a commit to mbland/tomcat-servlet-testing-example that referenced this issue Jan 13, 2024
Adds Typescript to devDependencies and a jsconfig.json files to ensure
Visual Studio Code performs more thorough type checking.

Description largely copied from:

- mbland/test-page-opener#22
  mbland/test-page-opener@a63f274
- mbland/test-page-opener#23
  mbland/test-page-opener@01a79f6
- mbland/jsdoc-cli-wrapper#20
  mbland/jsdoc-cli-wrapper@fafcd21
- mbland/rollup-plugin-handlebars-precompiler#7
  mbland/rollup-plugin-handlebars-precompiler@eb5b9a8
- mbland/rollup-plugin-handlebars-precompiler#8
  mbland/rollup-plugin-handlebars-precompiler@8b36b2a

The code is still JavaScript, but now we get strict type checking in
Visual Studio Code and in continuous integration via `tsc` in `pnpm
typecheck`.

The docs generated by 'jsdoc' are a little funky, and we don't get as
much documentation in Visual Studio Code as I expected. I believe I can
fix these issues at some point with this foundation in place.

The actual changes include:

- Added @types/chai, jsdoc, and typescript as devDependencies.

- Set .eslintrc to disable the no-undefined-types rule by extending
  "plugin:jsdoc/recommended-typescript-flavor-error". This is because
  the Handlebars types in lib/parser.js weren't trivial to replicate,
  and TypeScript finds those types just fine. This was based on advice
  from:

  > ...the config plugin:jsdoc/recommended-typescript-error should
  > disable the jsdoc/no-undefined-types rule because TypeScript itself
  > is responsible for reporting errors about invalid JSDoc types.
  >
  > - gajus/eslint-plugin-jsdoc#888 (comment)

  And:

  > If you are not using TypeScript syntax (your source files are still
  > .js files) but you are using the TypeScript flavor within JSDoc
  > (i.e., the default "typescript" mode in eslint-plugin-jsdoc) and you
  > are perhaps using allowJs and checkJs options of TypeScript's
  > tsconfig.json), you may use:
  >
  > ```json
  > {
  >   "extends": ["plugin:jsdoc/recommended-typescript-flavor"]
  > }
  > ```
  >
  > ...or to report with failing errors instead of mere warnings:
  >
  > ```json

  More background:

  - https://github.com/gajus/eslint-plugin-jsdoc/blob/main/docs/rules/no-undefined-types.md
  - gajus/eslint-plugin-jsdoc#99
  - gajus/eslint-plugin-jsdoc#1098
  - jsdoc/jsdoc#1537

- Added `settings.jsdoc.preferredTypes.Object = "object"` to .eslintrc
  to enable "Object.<..., ...>" syntax in a JSDoc `@typedef`. Got rid of
  some extra whitespaces in .eslintrc, too.
  - https://github.com/gajus/eslint-plugin-jsdoc/blob/b60cbb027b03b4f6d509933b0dca8681dbe47206/docs/rules/check-types.md#why-not-capital-case-everything
  - https://github.com/gajus/eslint-plugin-jsdoc/blob/b60cbb027b03b4f6d509933b0dca8681dbe47206/docs/settings.md#settings-to-configure-check-types-and-no-undefined-types

- Added '.js' extension to all internal imports and added JSDoc comments
  everywhere reqired by `pnpm typecheck`.

- Added 'jsdoc-plugin-typescript' to the build to handle the TypeScript
  `import().Type` directives. This ended up pulling in the 'es-abstract'
  module, which blew up the pnpm-lock.yaml file. If I get an itch, I'll
  implement my own plugin one day and replace it.

- Updated `pnpm test-ci` to incorporate `pnpm jsdoc` and `pnpm
  typecheck`. Added 'jsdoc' to devDependencies to enable this.

- Added `null` checks everywhere reqired by `pnpm typecheck`. Added
  tests to cover all the `null` cases.

- Added globals.d.ts and a `/* global STRCALC_BACKEND */` ESLint comment
  to calculators.js to properly type check `globalThis.STRCALC_BACKEND`.
  Ironically, this required just referencing it as `STRCALC_BACKEND`
  without `globalThis`.

- Added a temporary components/template.d.ts containing the Handlebars
  Template() type declaration. Once I properly export those types from
  rollup-plugin-handlebars-precompiler, I'll remove it. (That plugin
  currently contains lib/template.d.ts, not types/template.d.ts.)

- Added a test for the `#missing app element` case in main.js by adding
  a new test/main-missing-app-div.test.js. I need to port it to
  mbland/test-page-opener to solve the coverage problem encountered in:

  - mbland/test-page-opener#23
    mbland/test-page-opener@01a79f6

  > - Added a new missing.html and "JsdomPageOpener > doesn't throw if
  >   missing app div" test case to cover new null check in
  >   test-modules/main.js.
  >
  >   This did, however, throw off Istanbul coverage, but not V8
  >   coverage. Running just the "doesn't throw" test case shows 0%
  >   coverage of main.js, even though the test clearly passes. My
  >   suspicion is that Istanbul can't associate the
  >   `./main.js?version=missing` import path from missing.html with the
  >   test-modules/main.js file.
  >
  >   So now `pnpm test:ci:jsdom` will use the V8 provider, and `pnpm
  >   test:ci:browser`, which doesn't use missing.html, will continue to
  >   use Istanbul. Each task outputs its own separate .lcov file which
  >   then gets merged into Coveralls.

- Updated `setupFetchStub()` to detect the type of the `body` argument
  and call `JSON.stringify()` itself if it's an `object`. This
  eliminated the need for most callers to call `JSON.stringify()`.

- Updated `StringCalculatorPage` with typing information and made it so
  that an empty object will stand in for `null` elements. This is
  playing loose with typing a bit, as any `null`s will cause errors
  showing unknown property access. But that seemed better than
  burdening all callers to do their own `null` checks or workarounds.

Of special note:

- Added the `instantiate()` parameter to Calculator.init() to inject a
  Handlebars Template() function. This enabled testing that a missing
  `#numbers` element was logged by Calculator.init().

  Tests for this and Calculator.#submitRequest() set up a console.error
  spy along with a callback for Vitest's vi.waitFor().

  I need to write a document and/or blog post about this as part of the
  Handlebars Component Pattern. (I just came up with that name while
  writing it.)
mbland added a commit to mbland/tomcat-servlet-testing-example that referenced this issue Jan 13, 2024
Adds Typescript to devDependencies and a jsconfig.json files to ensure
Visual Studio Code performs more thorough type checking.

Description largely copied from:

- mbland/test-page-opener#22
  mbland/test-page-opener@a63f274
- mbland/test-page-opener#23
  mbland/test-page-opener@01a79f6
- mbland/jsdoc-cli-wrapper#20
  mbland/jsdoc-cli-wrapper@fafcd21
- mbland/rollup-plugin-handlebars-precompiler#7
  mbland/rollup-plugin-handlebars-precompiler@eb5b9a8
- mbland/rollup-plugin-handlebars-precompiler#8
  mbland/rollup-plugin-handlebars-precompiler@8b36b2a

The code is still JavaScript, but now we get strict type checking in
Visual Studio Code and in continuous integration via `tsc` in `pnpm
typecheck`.

The docs generated by 'jsdoc' are a little funky, and we don't get as
much documentation in Visual Studio Code as I expected. I believe I can
fix these issues at some point with this foundation in place.

The actual changes include:

- Added @types/chai, jsdoc, and typescript as devDependencies.

- Set .eslintrc to disable the no-undefined-types rule by extending
  "plugin:jsdoc/recommended-typescript-flavor-error". This is because
  the Handlebars types in lib/parser.js weren't trivial to replicate,
  and TypeScript finds those types just fine. This was based on advice
  from:

  > ...the config plugin:jsdoc/recommended-typescript-error should
  > disable the jsdoc/no-undefined-types rule because TypeScript itself
  > is responsible for reporting errors about invalid JSDoc types.
  >
  > - gajus/eslint-plugin-jsdoc#888 (comment)

  And:

  > If you are not using TypeScript syntax (your source files are still
  > .js files) but you are using the TypeScript flavor within JSDoc
  > (i.e., the default "typescript" mode in eslint-plugin-jsdoc) and you
  > are perhaps using allowJs and checkJs options of TypeScript's
  > tsconfig.json), you may use:
  >
  > ```json
  > {
  >   "extends": ["plugin:jsdoc/recommended-typescript-flavor"]
  > }
  > ```
  >
  > ...or to report with failing errors instead of mere warnings:
  >
  > ```json

  More background:

  - https://github.com/gajus/eslint-plugin-jsdoc/blob/main/docs/rules/no-undefined-types.md
  - gajus/eslint-plugin-jsdoc#99
  - gajus/eslint-plugin-jsdoc#1098
  - jsdoc/jsdoc#1537

- Added `settings.jsdoc.preferredTypes.Object = "object"` to .eslintrc
  to enable "Object.<..., ...>" syntax in a JSDoc `@typedef`. Got rid of
  some extra whitespaces in .eslintrc, too.
  - https://github.com/gajus/eslint-plugin-jsdoc/blob/b60cbb027b03b4f6d509933b0dca8681dbe47206/docs/rules/check-types.md#why-not-capital-case-everything
  - https://github.com/gajus/eslint-plugin-jsdoc/blob/b60cbb027b03b4f6d509933b0dca8681dbe47206/docs/settings.md#settings-to-configure-check-types-and-no-undefined-types

- Added '.js' extension to all internal imports and added JSDoc comments
  everywhere reqired by `pnpm typecheck`.

- Added 'jsdoc-plugin-typescript' to the build to handle the TypeScript
  `import().Type` directives. This ended up pulling in the 'es-abstract'
  module, which blew up the pnpm-lock.yaml file. If I get an itch, I'll
  implement my own plugin one day and replace it.

- Updated `pnpm test-ci` to incorporate `pnpm jsdoc` and `pnpm
  typecheck`. Added 'jsdoc' to devDependencies to enable this.

- Added `null` checks everywhere reqired by `pnpm typecheck`. Added
  tests to cover all the `null` cases.

- Added globals.d.ts and a `/* global STRCALC_BACKEND */` ESLint comment
  to calculators.js to properly type check `globalThis.STRCALC_BACKEND`.
  Ironically, this required just referencing it as `STRCALC_BACKEND`
  without `globalThis`.

- Added a temporary components/template.d.ts containing the Handlebars
  Template() type declaration. Once I properly export those types from
  rollup-plugin-handlebars-precompiler, I'll remove it. (That plugin
  currently contains lib/template.d.ts, not types/template.d.ts.)

- Added a test for the `#missing app element` case in main.js by adding
  a new test/main-missing-app-div.test.js. I need to port it to
  mbland/test-page-opener to solve the coverage problem encountered in:

  - mbland/test-page-opener#23
    mbland/test-page-opener@01a79f6

  > - Added a new missing.html and "JsdomPageOpener > doesn't throw if
  >   missing app div" test case to cover new null check in
  >   test-modules/main.js.
  >
  >   This did, however, throw off Istanbul coverage, but not V8
  >   coverage. Running just the "doesn't throw" test case shows 0%
  >   coverage of main.js, even though the test clearly passes. My
  >   suspicion is that Istanbul can't associate the
  >   `./main.js?version=missing` import path from missing.html with the
  >   test-modules/main.js file.
  >
  >   So now `pnpm test:ci:jsdom` will use the V8 provider, and `pnpm
  >   test:ci:browser`, which doesn't use missing.html, will continue to
  >   use Istanbul. Each task outputs its own separate .lcov file which
  >   then gets merged into Coveralls.

- Updated `setupFetchStub()` to detect the type of the `body` argument
  and call `JSON.stringify()` itself if it's an `object`. This
  eliminated the need for most callers to call `JSON.stringify()`.

- Updated `StringCalculatorPage` with typing information and made it so
  that an empty object will stand in for `null` elements. This is
  playing loose with typing a bit, as any `null`s will cause errors
  showing unknown property access. But that seemed better than
  burdening all callers to do their own `null` checks or workarounds.

Of special note:

- Added the `instantiate()` parameter to Calculator.init() to inject a
  Handlebars Template() function. This enabled testing that a missing
  `#numbers` element was logged by Calculator.init().

  Tests for this and Calculator.#submitRequest() set up a console.error
  spy along with a callback for Vitest's vi.waitFor().

  I need to write a document and/or blog post about this as part of the
  Handlebars Component Pattern. (I just came up with that name while
  writing it.)
mbland added a commit to mbland/tomcat-servlet-testing-example that referenced this issue Jan 13, 2024
Adds Typescript to devDependencies and a jsconfig.json files to ensure
Visual Studio Code performs more thorough type checking.

Description largely copied from:

- mbland/test-page-opener#22
  mbland/test-page-opener@a63f274
- mbland/test-page-opener#23
  mbland/test-page-opener@01a79f6
- mbland/jsdoc-cli-wrapper#20
  mbland/jsdoc-cli-wrapper@fafcd21
- mbland/rollup-plugin-handlebars-precompiler#7
  mbland/rollup-plugin-handlebars-precompiler@eb5b9a8
- mbland/rollup-plugin-handlebars-precompiler#8
  mbland/rollup-plugin-handlebars-precompiler@8b36b2a

The code is still JavaScript, but now we get strict type checking in
Visual Studio Code and in continuous integration via `tsc` in `pnpm
typecheck`.

The docs generated by 'jsdoc' are a little funky, and we don't get as
much documentation in Visual Studio Code as I expected. I believe I can
fix these issues at some point with this foundation in place.

The actual changes include:

- Added @types/chai, jsdoc, and typescript as devDependencies.

- Set .eslintrc to disable the no-undefined-types rule by extending
  "plugin:jsdoc/recommended-typescript-flavor-error". This is because
  the Handlebars types in lib/parser.js weren't trivial to replicate,
  and TypeScript finds those types just fine. This was based on advice
  from:

  > ...the config plugin:jsdoc/recommended-typescript-error should
  > disable the jsdoc/no-undefined-types rule because TypeScript itself
  > is responsible for reporting errors about invalid JSDoc types.
  >
  > - gajus/eslint-plugin-jsdoc#888 (comment)

  And:

  > If you are not using TypeScript syntax (your source files are still
  > .js files) but you are using the TypeScript flavor within JSDoc
  > (i.e., the default "typescript" mode in eslint-plugin-jsdoc) and you
  > are perhaps using allowJs and checkJs options of TypeScript's
  > tsconfig.json), you may use:
  >
  > ```json
  > {
  >   "extends": ["plugin:jsdoc/recommended-typescript-flavor"]
  > }
  > ```
  >
  > ...or to report with failing errors instead of mere warnings:
  >
  > ```json

  More background:

  - https://github.com/gajus/eslint-plugin-jsdoc/blob/main/docs/rules/no-undefined-types.md
  - gajus/eslint-plugin-jsdoc#99
  - gajus/eslint-plugin-jsdoc#1098
  - jsdoc/jsdoc#1537

- Added `settings.jsdoc.preferredTypes.Object = "object"` to .eslintrc
  to enable "Object.<..., ...>" syntax in a JSDoc `@typedef`. Got rid of
  some extra whitespaces in .eslintrc, too.
  - https://github.com/gajus/eslint-plugin-jsdoc/blob/b60cbb027b03b4f6d509933b0dca8681dbe47206/docs/rules/check-types.md#why-not-capital-case-everything
  - https://github.com/gajus/eslint-plugin-jsdoc/blob/b60cbb027b03b4f6d509933b0dca8681dbe47206/docs/settings.md#settings-to-configure-check-types-and-no-undefined-types

- Added '.js' extension to all internal imports and added JSDoc comments
  everywhere reqired by `pnpm typecheck`.

- Added 'jsdoc-plugin-typescript' to the build to handle the TypeScript
  `import().Type` directives. This ended up pulling in the 'es-abstract'
  module, which blew up the pnpm-lock.yaml file. If I get an itch, I'll
  implement my own plugin one day and replace it.

- Updated `pnpm test-ci` to incorporate `pnpm jsdoc` and `pnpm
  typecheck`. Added 'jsdoc' to devDependencies to enable this.

- Added `null` checks everywhere reqired by `pnpm typecheck`. Added
  tests to cover all the `null` cases.

- Added globals.d.ts and a `/* global STRCALC_BACKEND */` ESLint comment
  to calculators.js to properly type check `globalThis.STRCALC_BACKEND`.
  Ironically, this required just referencing it as `STRCALC_BACKEND`
  without `globalThis`.

- Added a temporary components/template.d.ts containing the Handlebars
  Template() type declaration. Once I properly export those types from
  rollup-plugin-handlebars-precompiler, I'll remove it. (That plugin
  currently contains lib/template.d.ts, not types/template.d.ts.)

- Added a test for the `#missing app element` case in main.js by adding
  a new test/main-missing-app-div.test.js. I need to port it to
  mbland/test-page-opener to solve the coverage problem encountered in:

  - mbland/test-page-opener#23
    mbland/test-page-opener@01a79f6

  > - Added a new missing.html and "JsdomPageOpener > doesn't throw if
  >   missing app div" test case to cover new null check in
  >   test-modules/main.js.
  >
  >   This did, however, throw off Istanbul coverage, but not V8
  >   coverage. Running just the "doesn't throw" test case shows 0%
  >   coverage of main.js, even though the test clearly passes. My
  >   suspicion is that Istanbul can't associate the
  >   `./main.js?version=missing` import path from missing.html with the
  >   test-modules/main.js file.
  >
  >   So now `pnpm test:ci:jsdom` will use the V8 provider, and `pnpm
  >   test:ci:browser`, which doesn't use missing.html, will continue to
  >   use Istanbul. Each task outputs its own separate .lcov file which
  >   then gets merged into Coveralls.

- Updated `setupFetchStub()` to detect the type of the `body` argument
  and call `JSON.stringify()` itself if it's an `object`. This
  eliminated the need for most callers to call `JSON.stringify()`.

- Updated `StringCalculatorPage` with typing information and made it so
  that an empty object will stand in for `null` elements. This is
  playing loose with typing a bit, as any `null`s will cause errors
  showing unknown property access. But that seemed better than
  burdening all callers to do their own `null` checks or workarounds.

Of special note:

- Added the `instantiate()` parameter to Calculator.init() to inject a
  Handlebars Template() function. This enabled testing that a missing
  `#numbers` element was logged by Calculator.init().

  Tests for this and Calculator.#submitRequest() set up a console.error
  spy along with a callback for Vitest's vi.waitFor().

  I need to write a document and/or blog post about this as part of the
  Handlebars Component Pattern. (I just came up with that name while
  writing it.)
mbland added a commit to mbland/tomcat-servlet-testing-example that referenced this issue Jan 13, 2024
Adds Typescript to devDependencies and a jsconfig.json files to ensure
Visual Studio Code performs more thorough type checking.

Description largely copied from:

- mbland/test-page-opener#22
  mbland/test-page-opener@a63f274
- mbland/test-page-opener#23
  mbland/test-page-opener@01a79f6
- mbland/jsdoc-cli-wrapper#20
  mbland/jsdoc-cli-wrapper@fafcd21
- mbland/rollup-plugin-handlebars-precompiler#7
  mbland/rollup-plugin-handlebars-precompiler@eb5b9a8
- mbland/rollup-plugin-handlebars-precompiler#8
  mbland/rollup-plugin-handlebars-precompiler@8b36b2a

The code is still JavaScript, but now we get strict type checking in
Visual Studio Code and in continuous integration via `tsc` in `pnpm
typecheck`.

The docs generated by 'jsdoc' are a little funky, and we don't get as
much documentation in Visual Studio Code as I expected. I believe I can
fix these issues at some point with this foundation in place.

The actual changes include:

- Added @types/chai, jsdoc, and typescript as devDependencies.

- Set .eslintrc to disable the no-undefined-types rule by extending
  "plugin:jsdoc/recommended-typescript-flavor-error". This is because
  the Handlebars types in lib/parser.js weren't trivial to replicate,
  and TypeScript finds those types just fine. This was based on advice
  from:

  > ...the config plugin:jsdoc/recommended-typescript-error should
  > disable the jsdoc/no-undefined-types rule because TypeScript itself
  > is responsible for reporting errors about invalid JSDoc types.
  >
  > - gajus/eslint-plugin-jsdoc#888 (comment)

  And:

  > If you are not using TypeScript syntax (your source files are still
  > .js files) but you are using the TypeScript flavor within JSDoc
  > (i.e., the default "typescript" mode in eslint-plugin-jsdoc) and you
  > are perhaps using allowJs and checkJs options of TypeScript's
  > tsconfig.json), you may use:
  >
  > ```json
  > {
  >   "extends": ["plugin:jsdoc/recommended-typescript-flavor"]
  > }
  > ```
  >
  > ...or to report with failing errors instead of mere warnings:
  >
  > ```json

  More background:

  - https://github.com/gajus/eslint-plugin-jsdoc/blob/main/docs/rules/no-undefined-types.md
  - gajus/eslint-plugin-jsdoc#99
  - gajus/eslint-plugin-jsdoc#1098
  - jsdoc/jsdoc#1537

- Added `settings.jsdoc.preferredTypes.Object = "object"` to .eslintrc
  to enable "Object.<..., ...>" syntax in a JSDoc `@typedef`. Got rid of
  some extra whitespaces in .eslintrc, too.
  - https://github.com/gajus/eslint-plugin-jsdoc/blob/b60cbb027b03b4f6d509933b0dca8681dbe47206/docs/rules/check-types.md#why-not-capital-case-everything
  - https://github.com/gajus/eslint-plugin-jsdoc/blob/b60cbb027b03b4f6d509933b0dca8681dbe47206/docs/settings.md#settings-to-configure-check-types-and-no-undefined-types

- Added '.js' extension to all internal imports and added JSDoc comments
  everywhere reqired by `pnpm typecheck`.

- Added 'jsdoc-plugin-typescript' to the build to handle the TypeScript
  `import().Type` directives. This ended up pulling in the 'es-abstract'
  module, which blew up the pnpm-lock.yaml file. If I get an itch, I'll
  implement my own plugin one day and replace it.

- Updated `pnpm test-ci` to incorporate `pnpm jsdoc` and `pnpm
  typecheck`. Added 'jsdoc' to devDependencies to enable this.

- Added `null` checks everywhere reqired by `pnpm typecheck`. Added
  tests to cover all the `null` cases.

- Added globals.d.ts and a `/* global STRCALC_BACKEND */` ESLint comment
  to calculators.js to properly type check `globalThis.STRCALC_BACKEND`.
  Ironically, this required just referencing it as `STRCALC_BACKEND`
  without `globalThis`.

- Added a temporary components/template.d.ts containing the Handlebars
  Template() type declaration. Once I properly export those types from
  rollup-plugin-handlebars-precompiler, I'll remove it. (That plugin
  currently contains lib/template.d.ts, not types/template.d.ts.)

- Added a test for the `#missing app element` case in main.js by adding
  a new test/main-missing-app-div.test.js. I need to port it to
  mbland/test-page-opener to solve the coverage problem encountered in:

  - mbland/test-page-opener#23
    mbland/test-page-opener@01a79f6

  > - Added a new missing.html and "JsdomPageOpener > doesn't throw if
  >   missing app div" test case to cover new null check in
  >   test-modules/main.js.
  >
  >   This did, however, throw off Istanbul coverage, but not V8
  >   coverage. Running just the "doesn't throw" test case shows 0%
  >   coverage of main.js, even though the test clearly passes. My
  >   suspicion is that Istanbul can't associate the
  >   `./main.js?version=missing` import path from missing.html with the
  >   test-modules/main.js file.
  >
  >   So now `pnpm test:ci:jsdom` will use the V8 provider, and `pnpm
  >   test:ci:browser`, which doesn't use missing.html, will continue to
  >   use Istanbul. Each task outputs its own separate .lcov file which
  >   then gets merged into Coveralls.

- Updated `setupFetchStub()` to detect the type of the `body` argument
  and call `JSON.stringify()` itself if it's an `object`. This
  eliminated the need for most callers to call `JSON.stringify()`.

- Updated `StringCalculatorPage` with typing information and made it so
  that an empty object will stand in for `null` elements. This is
  playing loose with typing a bit, as any `null`s will cause errors
  showing unknown property access. But that seemed better than
  burdening all callers to do their own `null` checks or workarounds.

Of special note:

- Added the `instantiate()` parameter to Calculator.init() to inject a
  Handlebars Template() function. This enabled testing that a missing
  `#numbers` element was logged by Calculator.init().

  Tests for this and Calculator.#submitRequest() set up a console.error
  spy along with a callback for Vitest's vi.waitFor().

  I need to write a document and/or blog post about this as part of the
  Handlebars Component Pattern. (I just came up with that name while
  writing it.)
mbland added a commit to mbland/test-page-opener that referenced this issue Jan 16, 2024
Most of this message is copied from:

- mbland/jsdoc-cli-wrapper#21
  mbland/jsdoc-cli-wrapper@e0d287d
- mbland/rollup-plugin-handlebars-precompiler#7
  mbland/rollup-plugin-handlebars-precompiler@eb5b9a8
- mbland/rollup-plugin-handlebars-precompiler#8
  mbland/rollup-plugin-handlebars-precompiler@8b36b2a

After doing more research, it appears tsconfig.json is more broadly
supported by editors and IDEs than jsconfig.json. For example, IntelliJ
IDEA/WebStorm won't recognize it unless added to Settings > Editor >
File Types > TypeScript.

Related changes include:

- Fixed the problems with vitest.config.js and ci/vitest.config.js such
  that the `// @ts-nocheck` directive is no longer necessary. Used
  `...(configDefaults.coverage.exclude || [])` to avoid type errors due
  to configDefaults.coverage.exclude potentially being undefined.

- Moved a bunch of compiler options from the `pnpm typecheck` script
  into tsconfig.json.

- Added the `rimraf` npm to make sure `pnpm prepack` generates a new
  `types/` directory without stale content.

- Added @types/{jsdom,istanbul-lib-coverage} as production dependencies.

- Bumped vitest to 1.2.0.

- Set .eslintrc to disable the no-undefined-types rule by extending
  "plugin:jsdoc/recommended-typescript-flavor-error". This is because
  the Handlebars types in lib/parser.js weren't trivial to replicate,
  and TypeScript finds those types just fine. This was based on advice
  from:

  > ...the config plugin:jsdoc/recommended-typescript-error should
  > disable the jsdoc/no-undefined-types rule because TypeScript itself
  > is responsible for reporting errors about invalid JSDoc types.
  >
  > - gajus/eslint-plugin-jsdoc#888 (comment)

  And:

  > If you are not using TypeScript syntax (your source files are still
  > .js files) but you are using the TypeScript flavor within JSDoc
  > (i.e., the default "typescript" mode in eslint-plugin-jsdoc) and you
  > are perhaps using allowJs and checkJs options of TypeScript's
  > tsconfig.json), you may use:
  >
  > ```json
  > {
  >   "extends": ["plugin:jsdoc/recommended-typescript-flavor"]
  > }
  > ```
  >
  > ...or to report with failing errors instead of mere warnings:
  >
  > ```json
  > {
  >   "extends": ["plugin:jsdoc/recommended-typescript-flavor-error"]
  > }
  > ```
  >
  > - https://github.com/gajus/eslint-plugin-jsdoc#eslintrc

  More background:

  - https://github.com/gajus/eslint-plugin-jsdoc/blob/main/docs/rules/no-undefined-types.md
  - gajus/eslint-plugin-jsdoc#99
  - gajus/eslint-plugin-jsdoc#1098
  - jsdoc/jsdoc#1537

- At the same time, extending "recommended-typescript-flavor-error"
  required adding the `// eslint-disable-next-line no-unused-vars`
  directive before each set of imports from lib/types.js. (Or adding
  `// eslint-disable-line no-unused-vars` on the same line if possible.)

- Added 'jsdoc-plugin-typescript' to the build to handle the TypeScript
  `import().Type` directives. This ended up pulling in the 'es-abstract'
  module, which blew up the pnpm-lock.yaml file. If I get an itch, I'll
  implement my own plugin one day and replace it.

- Finally, it seems IntelliJ IDEA's JSDoc type checking is stronger than
  TypeScript and VSCode. Fixed a few JSDoc type parameters to eliminate
  warnings in IntelliJ as well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants