Pwa-studio: [feature]: Create an own @types package that contains type definitions

Created on 16 Feb 2020  Â·  6Comments  Â·  Source: magento/pwa-studio

Is your feature request related to a problem? Please describe.

To support TypeScript it is necessary to have an own types package for peregrine and venia-ui. React and other Libraries have an own @types package that holds the type definition.

Describe the solution you'd like
I want to use TypeScript for own components that based on venia-ui but then I also need to write my own types

Describe alternatives you've considered

Additional context

Please let us know what packages this feature is in regards to:

  • [ ] venia-concept
  • [x] venia-ui
  • [ ] pwa-buildpack
  • [x] peregrine
  • [ ] pwa-devdocs
  • [ ] upward-js
  • [ ] upward-spec
  • [ ] create-pwa
ready for grooming enhancement help wanted

All 6 comments

@jimbo to review

Related #1585

Hi Guys! I’ve been working on generating typescript declarations for the peregrine and venia-ui packages and was wondering how to proceed in contributing them back to the project?

Especially in the sense that I don’t know what the best solution would be how these types should be integrated. Should they be distributed throughout the project (close to the actual files), should they be separate packages, be generated with a GitHub action, or?

Also, what is the way forward in regards to typescript, it generally is considered is the way forward for big javascript packages. Is Magento considering this and willing to migrate to a more typescript friendly environment?

There can be multiple strategies:

  • Generate .d.ts based on javascript doc types (that is how open-wc functions for example https://github.com/open-wc/open-wc/blob/master/packages/scoped-elements/src/registerElement.js)
  • Create .d.ts files manually and generate proptypes automatically from that https://github.com/mui-org/material-ui/blob/master/packages/material-ui/src/Badge/Badge.js#L214
  • Start with the generated .d.ts files and maintain them manually from there, might be ‘easier’ as d.ts files are easier to write than writing javascript doc types.

@paales I will add this to our internal meeting today

I was for example able to generate the following for @magento/venia-ui/lib/components/ProductFullDetail

export default ProductFullDetail
declare function ProductFullDetail(props: any): JSX.Element
declare namespace ProductFullDetail {
  export namespace propTypes {
    export const classes: import('prop-types').Requireable<import('prop-types').InferProps<{
      cartActions: import('prop-types').Requireable<string>
      description: import('prop-types').Requireable<string>
      descriptionTitle: import('prop-types').Requireable<string>
      details: import('prop-types').Requireable<string>
      detailsTitle: import('prop-types').Requireable<string>
      imageCarousel: import('prop-types').Requireable<string>
      options: import('prop-types').Requireable<string>
      productName: import('prop-types').Requireable<string>
      productPrice: import('prop-types').Requireable<string>
      quantity: import('prop-types').Requireable<string>
      quantityTitle: import('prop-types').Requireable<string>
      root: import('prop-types').Requireable<string>
      title: import('prop-types').Requireable<string>
    }>>
    export const product: import('prop-types').Validator<import('prop-types').InferProps<{
      __typename: import('prop-types').Requireable<string>
      id: import('prop-types').Requireable<number>
      sku: import('prop-types').Validator<string>
      price: import('prop-types').Validator<
        import('prop-types').InferProps<{
          regularPrice: import('prop-types').Validator<
            import('prop-types').InferProps<{
              amount: import('prop-types').Requireable<
                import('prop-types').InferProps<{
                  currency: import('prop-types').Validator<string>
                  value: import('prop-types').Validator<number>
                }>
              >
            }>
          >
        }>
      >
      media_gallery_entries: import('prop-types').Requireable<
        import('prop-types').InferProps<{
          label: import('prop-types').Requireable<string>
          position: import('prop-types').Requireable<number>
          disabled: import('prop-types').Requireable<boolean>
          file: import('prop-types').Validator<string>
        }>[]
      >
      description: import('prop-types').Requireable<string>
    }>>
  }
}

Which results in a fully working typescript declarations with strict typescript validation. Of course there are holes in lots of typescript declarations and there are lot of any types in the created definitions for methods/functions with non-component files.

The generation is based on this https://www.typescriptlang.org/docs/handbook/declaration-files/dts-from-js.html and seems to be working fairly well.

Example of the result:
Schermafbeelding 2020-05-26 om 14 56 50

Since communication happened on Slack and over Bluejeans I'll try and state my points:

If you're developing PWA Studio with Visual Studio Code, you are already getting the benefits of TypeScript: VSCode's IntelliSense is being fed by the TypeScript compiler.

What IntelliSense doesn't do is: evaluate all javascript modules inside the node_modules folder, because that would be way to heavy. It can read d.ts files to get the same developer experience as when building the package yourself.

The feature request isn't so much to have 100% typescript coverage and that shouldn't be the goal, the goal is to offer 90% without much effort.

Create a DefinitelyTyped package: This is more difficult to maintain as everything needs to be in sync for this to work, this would place a huge burden on the maintainer of that package. With some release automation on this package it can be implemented fairly automatically and the types can be part a build step while publishing to npm.

Since installing @types packages is completely optional, there wouldn't be any additional maintenance overhead with them once they are set up.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

brendanfalkowski picture brendanfalkowski  Â·  4Comments

sruthitechfriar picture sruthitechfriar  Â·  4Comments

dambrogia picture dambrogia  Â·  7Comments

senthil134 picture senthil134  Â·  7Comments

supernova-at picture supernova-at  Â·  4Comments