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

feat(types): add aria attributes and refactor type gen #4

Merged
merged 6 commits into from Mar 28, 2022

Conversation

willmartian
Copy link
Owner

@willmartian willmartian commented Mar 28, 2022

This PR refactors the type generation logic found in the core package.

Key updates:

  • Aria attributes are now recognized and do not cause a type error ( resolves Allow aria attributes #2 ). Short term work still needs to be done to add JSDoc descriptions to each attribute, and refine their value type. Less short term, it would also be nice to restrict the allowed aria attributes to those that are compatible with the role of the element (is this possible?).

  • Lerna mode was swapped to independent. This PR started out with the intention to move HTML types to their own package, and with that, I decided independent mode made a bit more sense for the repo. However, I changed my mind on splitting the types into a separate package due to some limitations with mapped types preserving JSDoc comments and tags. Ideally, I would like for more of the manual type generate to utilize TS itself over code gen, but at this point the IDE type hints are better if we are less DRY (sadly).

  • More investigation needs to be done on "convenience patterns", i.e. passing Array<string> as the value to .class(). The behavior needs to be consistent--I think one off special cases are a potential footgun and I am removing for now ( concerns Type of class attribute allows array of strings #3 ). Should any of the convenience patterns be built in, or moved to a plugin? I am thinking plugins, but then, what is the best way for users to update the accepted types of the h function?

    Prior art here, but it would involve changing the api in a way that requires constructing a new instance of h before it is usable: const h = new hdot().

@netlify
Copy link

netlify bot commented Mar 28, 2022

Deploy Preview for hdotjs ready!

Name Link
🔨 Latest commit 14e251a
🔍 Latest deploy log https://app.netlify.com/sites/hdotjs/deploys/624136a36eb1e7000855de20
😎 Deploy Preview https://deploy-preview-4--hdotjs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@willmartian willmartian merged commit 0d0d67d into main Mar 28, 2022
@willmartian willmartian deleted the refactor-types branch March 28, 2022 04:22
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.

Allow aria attributes
1 participant