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

pre-load manifests #20

Open
bennypowers opened this issue May 3, 2023 · 3 comments
Open

pre-load manifests #20

bennypowers opened this issue May 3, 2023 · 3 comments

Comments

@bennypowers
Copy link

Say I know in advance that i'm working with a certain cem, like my company's design system.
I want to configure ce-ls to always load that manifest up, by URL

possible config:

require'lspconfig'.custom_elements_ls.setup {
  preload = {
    cache_expiry = '1w',
    manifests = {
      'https://unpkg.com/@patternfly/elements/custom-elements.json',
    },
  }
}
@Matsuuu
Copy link
Owner

Matsuuu commented May 3, 2023

Heya @bennypowers !

This is a great suggestion. It kind of reminds me of that JDTLS does with completion priorization.

The only downside I see here is that you'll be providing a manifest to use in all of your projects, which will mean that you will need to have the packages installed on your local machine, or have to imports point to the same CDN.

So a couple of questions on the behavior:

Should it...

  1. Not provide this action to begin with
  2. Provide the code action and import import 'https://unpkg.com/@patternfly/[email protected]/pf-accordion/pf-accordion-header.js'
  3. Expect that the package is installed locally and import import '@patternfly/[email protected]/pf-accordion/pf-accordion-header.js

?

@bennypowers
Copy link
Author

bennypowers commented May 3, 2023

Well, here's my instigating scenario:

<!DOCTYPE html>
<html lang="en">
  <head>

    <script type="importmap">
    {
      "imports": {
        "@patternfly/elements/": "https://jspm.dev/@patternfly/elements/"
      }
    }
    </script>

    <script type="module">
      import '@patternfly/elements/pf-card/pf-card.js';
      import '@patternfly/elements/pf-switch/pf-switch.js';
      import '@patternfly/elements/pf-label/pf-label.js';
      import '@patternfly/elements/pf-tooltip/pf-tooltip.js';
    </script>

  </head>
  <body>

    <pf-card size="large">
      <h1 slot="header">PatternFly Elements</h1>
      <p>
        A performant design system, made with
        <pf-label color="blue"></pf-label> and
        <pf-tooltip content="Performant, progressively-enhanced experiences">
          <pf-label color="orange">Web Components</pf-label>
        </pf-tooltip>
      </p>
      <label slot="footer">Rounded?
        <pf-switch onchange="this.closest('pf-card').rounded = this.checked"></pf-switch>
      </label>
      <label slot="footer">Compact?
        <pf-switch onchange="this.closest('pf-card').size = this.checked ? 'compact' : 'large'"></pf-switch>
      </label>
    </pf-card>


  </body>
</html>

Just reading through this html file, I can imagine that the LS should be able to fetch the pkg json from the cdn (happens to be that jspm.dev doesn't serve the package.json or the custom-elements.json unless you specifically ask it to, and unpkg doesn't resolve package exports, but let's ignore those problems for now), read the customElements key in the package.json and load up the manifest.

Alternatively, if the manifest is pre-loaded, the ls could just notice that the package is imported in the <head> - perhaps preload.manifests could be a table of string or tuple, like

require'lspconfig'.custom_elements_ls.setup {
  preload = {
    cache_expiry = '1w',
    manifests = {
      'https://unpkg.com/@shoelace-style/shoelace/custom-elements.json',
      { '@patternfly/elements', 'https://unpkg.com/@patternfly/elements/custom-elements.json', },
    },
  }
}

In any event, in cases where the manifest is present but the sources are not (or can't be derived from the manifest), just don't perform that action.

In the case of manifests loaded by URL - perhaps the ls could open a browser tab to the file e.g. https://unpkg.com/@patternfly/elements/pf-switch/pf-switch.js. In cases where an import map is present and parsed in the file, the ls can probably calculate a URL for the source file from the import map

@Matsuuu
Copy link
Owner

Matsuuu commented May 3, 2023

Thanks for the thorough response.

I can see the situation now and something like this would be quite beneficial. Something I'm a bit sceptical on is calling the internet from the language server as I feel that a lot of language tools should work on local machines without having to interface with the web.

Then again, Custom Elements are purely web tech, so calling the web is pretty much inevitable, and this being an opt in feature, I don't see a reason not to implement something like this.

I will definitely put this on the roadmap, and will get back to you on how this is going on in the coming weeks. Currently I'm working on some major changes to how the typescript instance is handled and how CEM's are analyzed / managed, so those will be coming in first, but I'll be keeping this in mind while I work on the analyzer integration

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