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

Add maxNodeModuleJsDepth to the default lit-analyzer tsconfig #122

Open
abdonrd opened this issue Jul 20, 2020 · 5 comments
Open

Add maxNodeModuleJsDepth to the default lit-analyzer tsconfig #122

abdonrd opened this issue Jul 20, 2020 · 5 comments

Comments

@abdonrd
Copy link
Contributor

abdonrd commented Jul 20, 2020

After reading the no-unknown-tag-name rule...
What's the way to add the "@CustomElement tag-name" JSDoc?

Right now I have this issue (IBM/pwa-lit-template#12) with this components (pwa-helper-components).
Another reference: thepassle/pwa-helpers#12

If I update this file:

https://github.com/thepassle/pwa-helpers/blob/8a761a5ea11e29e44c986c6eb2f8f26b5acdd197/pwa-install-button.js#L1-L3

To be like:

import { PwaInstallButton } from './pwa-install-button/PwaInstallButton.js';

/**
 * @customElement pwa-install-button
 */
customElements.define('pwa-install-button', PwaInstallButton);

Should be enough? I can't get it to work.

@runem
Copy link
Owner

runem commented Jul 20, 2020

Thanks for reporting this, and thanks for the great explanation :-)

TLDR: You can fix this pretty easily by adding "maxNodeModuleJsDepth": 3 to your tsconfig.json.

First of all @customElement pwa-install-button should be on the class declaration like this:

/**
 * @customElement pwa-install-button
 */
export class PwaInstallButton extends HTMLElement {
}

But you don't need that, because actually there is no problem with the pwa-install-button dependency because it can be analyzed by web-component-analyzer. You can see this for yourself by running npx web-component-analyzer node_modules/pwa-helper-components/pwa-install-button.js --discoverNodeModules from the root of your project.

Your problem comes from another, more essential, challenge which affects how to consume/analyze dependencies. I described this in details in your other issue here. lit-analyzer is built upon the Typescript parser which expects node_modules to contain type definition files announcing elements by extending HTMLElementTagNameMap or with declarations with the @customElement JSDoc tag. I don't expect all modules to have type definition files, but this comes with some challenges, because lit-analyzer (thus the Typescript parser which it builds upon) doesn't analyze external Javascript imports out of the box. It doesn't do this because analyzing all external dependencies would be too slow. If you run lit-analyzer with the --debug flag you can see exactly which files lit-analyzer analyzes.

In your file app-index.ts you are importing pwa-install-button using import 'pwa-helper-components/pwa-install-button.js';. This file unfortunately doesn't get analyzed because it's an external Javascript file. You can change this yourself by using the maxNodeModuleJsDepth config in your tsconfig.json file and fix problem.

For libraries where the author doesn't ship type definition files and for projects that don't use a tsconfig.json file, there are two long term solutions two this problem, both of which I haven't implemented yet:

  1. The first solution is for dependencies to ship with a custom-element.json file. This format hasn't been "standardized" yet, and I haven't introduced support for this file yet. This solution has the disadvantage that it requires the author of a library to generate and ship this file.
  2. Another solution is for me to begin analyzing external Javascript modules out of the box. I think it could end up with a settings where the analyzed depth of externally Javascript modules can be specified when running the CLI (by setting maxNodeModuleJsDepth TS config automatically). This solution has the disadvantage that it would only work when running the analyzer from the lit-analyzer CLI and wouldn't work within VSCode unless the user has a tsconfig.json file with maxNodeModuleJsDepth specified.

Until some of this has been implemented, you can either chose to add maxNodeModuleJsDepth to your tsconfig.json, mute the warning or tell lit-analyzer that the tag names exist.

Muting the warning:

Telling lit-analyzer that the tag names exist

  • Add the following to app-index.ts:
declare global {
  interface HTMLElementTagNameMap {
    'pwa-install-button': HTMLElement;
    'pwa-update-available': HTMLElement;
  }
}

@abdonrd
Copy link
Contributor Author

abdonrd commented Jul 21, 2020

Wow, thanks for this complete explanation! 👏

I have two questions:

  • What are the implications of adding the "maxNodeModuleJsDepth": 3?
    Beyond fixing this particular problem.
    And also I need to add the "allowJs": true, right?
    And it also works with "maxNodeModuleJsDepth": 2

  • Telling lit-analyzer that the tag names exist like this would be better?
    Although Could not find a declaration file for module error appears.

import { PwaInstallButton } from 'pwa-helper-components/pwa-install-button/PwaInstallButton.js';
import { PwaUpdateAvailable } from 'pwa-helper-components/pwa-update-available/PwaUpdateAvailable.js';

declare global {
  interface HTMLElementTagNameMap {
    'pwa-install-button': PwaInstallButton;
    'pwa-update-available': PwaUpdateAvailable;
  }
}

@runem
Copy link
Owner

runem commented Jul 23, 2020

The implication is that tsc will be a bit slower due to parsing some of the javascript dependencies. If you run lit-analyzer with the --debug flag, you can see exactly which files are included in the tsc program. And yes, I think "allowJs": true is also required for this to work. Also, it makes sense that a depth 2 would also work, because the dependencies are only 2 levels deep: pwa-helper-components/pwa-install-button.js ==> pwa-helper-components/pwa-install-button/PwaInstallButton.js.

I think both of the methods are equally fine, and it really depends on if you want to type the elements yourself (for example, add some properties to the PwaInstallButton that lit-analyzer didn't find based on the analysis of the javascript files).

The reason you get Could not find a declaration file for module is that there are no typings shipped with pwa-helper-components, so Typescript doesn't know the content of the file. I don't even think maxNodeModuleJsDepth can fix this. You can either choose to to add those typings yourself in a separate typings file (index.d.ts) in your own project:

declare module "pwa-helper-components/pwa-install-button/PwaInstallButton.js" {
  /**
   * @fires {CustomEvent<boolean>} pwa-installable
   * @fires {CustomEvent<boolean>} pwa-installed
   */
  export class PwaInstallButton {}
}

Or you can choose to just use HTMLElement instead like this:

declare global {
  interface HTMLElementTagNameMap {
    'pwa-install-button': HTMLElement;
  }
}

@runem
Copy link
Owner

runem commented Jul 23, 2020

Is it okay for you if I change the title of this issue to something like: "Add maxNodeModuleJsDepth to the default lit-analyzer tsconfig"? :-)

@abdonrd
Copy link
Contributor Author

abdonrd commented Jul 24, 2020

Just with the:

    "allowJs": true,
    "maxNodeModuleJsDepth": 2,

It finds the components:

Screenshot 2020-07-24 at 13 52 06

Screenshot 2020-07-24 at 13 54 45

@abdonrd abdonrd changed the title Fix no-unknown-tag-name rule with @customElement tag-name JSDoc Add maxNodeModuleJsDepth to the default lit-analyzer tsconfig Jul 24, 2020
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

No branches or pull requests

2 participants