search

open-wc / custom-elements-manifest-deprecated

Custom Elements Manifest is a file format that describes custom elements in your project.
8 stars 4 forks source link

Remove any types and implement proper type assertions #16

Closed AMoldskred closed 3 years ago

AMoldskred commented 3 years ago

In this PR I have started to remove any-types by creating proper type-asssertions. I have also moved some conditionals to make it both easier to read and for typescript to easier infer the correct type.

It is not a complete fix, but it should be a good start. Before merging with master I would try to pull locally and test that it doesn't break anything. I couldn't figure out how to use the correct ts-lint and how to run it. But I am fairly certain that it should be all good

14

thepassle commented 3 years ago

This is really great! Very valuable help here, I appreciate it a lot 🙂

It does seem like theres some breakage regarding the types however: image

Could you take a look? You can confirm the types are working by, from the package root:

yarn install
cd packages/custom-elements-json-core
npm run tsc:watch
AMoldskred commented 3 years ago

Will do!

AMoldskred commented 3 years ago

@thepassle I'm not going to lie. There needs some serious work done to make typing to work properly in this project. I have done what I can for now to fix the typing, however there is one place where I had to force any type in src/ast/handleEvents (There was no logical way that I could see that would infer the correct type).

AMoldskred commented 3 years ago

@thepassle It would be really great if you could tag this PR with hacktoberfest-accepted so that I could get this as a PR towards my 4 in hacktoberfest😉

thepassle commented 3 years ago

@thepassle It would be really great if you could tag this PR with hacktoberfest-accepted so that I could get this as a PR towards my 4 in hacktoberfest😉

Sorry for the late reply, I've had some IRL stuff to attend to this past week, and I've also been considering the approach of this project, and potentially just moving it over to JS with JSDoc, but I'm still unsure on what to do.

Does adding a hacktoberfest-accepted immediately count towards your hacktober fest progress? Even if this doesn't get merged? If so, I'll happily add it so you can complete your hacktoberfest goal.

AMoldskred commented 3 years ago

Thank you @thepassle ! The PR started counting as soon as you added the label. I wish you best of luck with your project!