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 Web-Types generation support #239

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open

Conversation

jpradelle
Copy link

Add Web-Types transformer to generate web-types files.

Web-Types is a project to enable IDE auto-completion for web-components, currently implemented in IntelliJ and WebStorm: https://github.com/JetBrains/web-types

For example after generating web-types file with web-component-analyzer
image

@marcobeierer
Copy link

@runem Thank you for your great work.

Quick question: Are you interested in adding support for web types or would your prefer if we maintain a fork?

@jpradelle
Copy link
Author

Currently a release of fork is available here https://www.npmjs.com/package/web-component-analyzer-webtypes
I would rather have it merged in this branch :) I'm currently using and maintaining the fork

@marcobeierer
Copy link

@jpradelle Thank you for the info, good to know 👍

@minijus
Copy link

minijus commented Aug 11, 2022

Would any of owners/contributors be able to review this awesome PR?
@rictic @43081j @runem

@43081j
Copy link
Contributor

43081j commented Aug 11, 2022

ill try have a read through it this weekend if i can, and get my head around it.

interesting that jetbrains chose to build their own schema rather than use custom elements manifest. maybe it didn't exist when they made it.

@piotrtomiak
Copy link

piotrtomiak commented Aug 25, 2022

@43081j Web-Types were in development much longer than the custom-elements-manifest and are more generic as they are not tied to a particular framework. Unfortunately we haven't made it in time before custom-elements-manifest was finished. Besides for Web Components, the format is used for Vue.js, Angular, HTMX and others. It also allows pattern processing, so works with for instance Fontawesome library.

It would be awesome if the PR is merged! I will then update documentation in the web-types GitHub repo on how to generate web-types for Web Components.

Copy link

@piotrtomiak piotrtomiak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jpradelle Awesome work, thanks a lot! My only suggestion would be to take name and version from package.json as a fallback. Also, please have a look at work of @Atulin - jpradelle/web-component-analyzer@master...Atulin:web-component-analyzer:master , might be worth including these commits in the PR.

@jpradelle
Copy link
Author

@piotrtomiak Great suggestion, I'll have a look to integrate that.
I'll also take @Atulin work which is great, but I want to preserve a way to configure parameter in command line too, I'll need to do some adaptation: I build webtypes during release process, it needs to be build it with the next version number that will be released at end of release process

README.md Outdated
@@ -1,262 +1,289 @@
<h1 align="center">web-component-analyzer</h1>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

im guessing this entire-file-changed diff is because of line endings? do you know what we went from/to?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comes from pre-commit hook that did this change.
If you do a minor change in README and commit it, this should do the same. (I was able reproduce on a fresh clone, I hope it's not related to environment, Debian Linux on my case)
I think you should either commit line endings to README on master branch or update commit hook to avoid doing this change.

@jpradelle
Copy link
Author

@piotrtomiak I made requested update taking inspiration from @Atulin work, configuration of projects is cleaner this way :)
For example: jpradelle/polymer-web-types@4996ba7#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519

I did not read name or version from package.json, I took it from environment variable added by npm when you run a script from npm run, so if you don't run it from npm scripts package section this doesn't find name of version and it has to be provided via command line or package.json wca configuration. I think it does the job even if it's not perfect. I did that to avoid adding new dependencies to project. I would be happy if someone has a better way to suggest

@jpradelle
Copy link
Author

For clean support of PolymerJS web comopnents, please also consider merging this PR #238

doc/web-types.md Outdated Show resolved Hide resolved
README.md Outdated
<!-- prettier-ignore -->
| Option | Type | Description |
|-----------------------------|--------------------------| ---------------------------------------------------------------------------- |
| `--format <format>` | `markdown` \ | `json` \| `vscode` \| `webtypes` | Specify output format. Default is `markdown`. |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this got a bit mangled at some point. the \ isn't escaping the vertical bar anymore since there's newly added whitespace after it. so the type is in the wrong column

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you are right, I broke README. Full file end lines got changed changed by pre-commit hook, and also IDE rebuilt table formatting without understanding | column escaping, which has not been seen due to full file line ending change.

Can you please commit line ending change or disable line ending change on pre-commit hook on README on master branch ? Currently whatever the change I make, full README comes in change.
I will then remove any change on README not related to web-types.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for now i would check what the line endings are in the existing file and keep them that way, then fix up what your editor did to the columns here.

even if you reformatted it to LF (assuming thats what happened here, CRLF -> LF), it'd be fine as long as you don't mangle the table and what not.

this PR is gonna take some time to land as the only other maintainers are pretty busy most of the time. so depending on another PR to change formatting etc isn't gonna be ideal i think (though we may still need to do that separately).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was able to avoid to avoid commit hook changing line endings by removing package.json and node_modules before commiting. Readme is fixed, with a clean diff with master branch

@43081j
Copy link
Contributor

43081j commented Aug 29, 2022

i haven't had time to try it myself but what i've read so far seems fine, makes sense.

it'd be really, really nice if you could add some tests though 👀

and ideally we need a review from @rictic if he gets some spare time

@jpradelle
Copy link
Author

I added several tests

@minijus
Copy link

minijus commented Dec 5, 2022

Any update on this PR? Would be lovely to have a review by code owners to get support for web-types merged and released.

@43081j
Copy link
Contributor

43081j commented Dec 25, 2022

Sorry it took so long to reply here.

I can re-review it (and will) but I think we need Peter or someone else from the lit team to review/merge. I'll see if I can pair with someone from the team in the new year and get things moving

@m4olivei
Copy link

Any updates here? Would love to see this land. I've used a fork of the fork mentioned here with success:
https://github.com/tevey/web-component-analyzer

@43081j
Copy link
Contributor

43081j commented Jul 13, 2024

This repo is slow to update, and the alternative analyzer by the lit team doesn't yet have a vscode plugin etc

I think it'll be up to me or @justinfagnani to update if either of us get some time. Or we somehow find time to finish the official one and do this there instead

@m4olivei
Copy link

@43081j where is the official one?

@43081j
Copy link
Contributor

43081j commented Jul 15, 2024

You can find it here:
https://github.com/lit/lit/tree/main/packages/labs/analyzer

There are plans to build an official vscode extension but work hasn't started on it yet

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

Successfully merging this pull request may close these issues.

6 participants