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 TypeScript types #12

Open
SaadBazaz opened this issue Aug 5, 2024 · 10 comments
Open

Add TypeScript types #12

SaadBazaz opened this issue Aug 5, 2024 · 10 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@SaadBazaz
Copy link
Member

SaadBazaz commented Aug 5, 2024

We currently need to add //@ts-nocheck because TypeScript types don't work :(

For some reason, when we compile with tsup with dts: true, it throws weird errors. (Note that this line is commented out)

Also, our other non-Web-Component files need TypeScript types too.

These also need to be consistent.

@SaadBazaz SaadBazaz added enhancement New feature or request help wanted Extra attention is needed labels Aug 5, 2024
@alienkarma
Copy link

alienkarma commented Aug 22, 2024

Hi
I would like to work on this project.
I will try working on the types and update once done.

@SaadBazaz
Copy link
Member Author

Hi I would like to work on this project. I will try working on the types and update once done.

Hey @alienkarma, this is a critical issue for the library. Thanks for taking it up.

Let me know if you have any questions!

@alienkarma
Copy link

Hi
I am facing this error when I try to run dev.


material-web-components-react:dev:  ELIFECYCLE  Command failed with exit code 1.
material-web-components-react:dev: ERROR: command finished with error: command (O:\Documents\GitHub\public\material-web-components-react\packages\ui) C:\Program Files\nodejs\pnpm.cmd run dev exited (1)
material-web-components-react#dev: command (O:\Documents\GitHub\public\material-web-components-react\packages\ui) C:\Program Files\nodejs\pnpm.cmd run dev exited (1)

 Tasks:    0 successful, 2 total
Cached:    0 cached, 2 total
  Time:    1.599s
Failed:    material-web-components-react#dev

 ERROR  run failed: command  exited (1)
 ELIFECYCLE  Command failed with exit code 1.

I installed pnpm, and then ran the following commands from the root project:-
pnpm install
pnpm run dev

Am I doing something wrong? Do I need to configure anything else?

Also looking at the structure, inside the packages/ui folder, I will create a types folder and place all the type declarations there.

@alienkarma
Copy link

alienkarma commented Aug 24, 2024

@SaadBazaz Hi
Let me know if you can help me with this.

@SaadBazaz
Copy link
Member Author

@alienkarma - are you running on Windows?

This works okay on *nix machines.

@SaadBazaz
Copy link
Member Author

Also, from your error log, it seems like you don't have pnpm installed globally.
Check these docs to learn how to.

Also, I recommend WSL for development.

@alienkarma
Copy link

Hi
I have gone through the code and was trying to work on implementing types for the components.
Unfortunately, after a lot of tries and research, I don't think there is a good way to implement types for the react version of material web components.
The primary issue is all the attributes might exist between each other but are wrongly typed from the get-go.
(For example. value field accepts only string in web components but accepts string, number, or null in the react component)
This means that fixing your components to implement proper types would involve creating each component from scratch and properly typing each attribute and it is not feasible (at least by me).
I wish you luck with this project and apologize for not being of much help.

@SaadBazaz
Copy link
Member Author

Hi I have gone through the code and was trying to work on implementing types for the components. Unfortunately, after a lot of tries and research, I don't think there is a good way to implement types for the react version of material web components. The primary issue is all the attributes might exist between each other but are wrongly typed from the get-go. (For example. value field accepts only string in web components but accepts string, number, or null in the react component) This means that fixing your components to implement proper types would involve creating each component from scratch and properly typing each attribute and it is not feasible (at least by me). I wish you luck with this project and apologize for not being of much help.

I understand, thanks for the attempt. There's a lot more todo in the project too, you can check the Pull Requests.

A different approach for this particular issue:
Couldn't we just import the types from @material/web, and wrap each component and add the type on it?

@alienkarma
Copy link

I am not sure if that would work or not. I will try to rework on it using what you suggested. Will update once I get to it.

@SaadBazaz
Copy link
Member Author

@alienkarma - Did you have any progress? Could you open a PR with the progress you have?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants