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

fix(types): generate types for dist-custom-elements #3270

Merged
merged 4 commits into from
Mar 16, 2022

Conversation

rwaskiewicz
Copy link
Member

@rwaskiewicz rwaskiewicz commented Mar 8, 2022

Pull request checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been reviewed and added / updated if needed (for bug fixes / features)
  • Build (npm run build) was run locally and any changes were pushed
  • Unit tests (npm test) were run locally and passed
  • E2E Tests (npm run test.karma.prod) were run locally and passed
  • Prettier (npm run prettier) was run locally and passed

Pull request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Type declarations are not created for users of the dist-custom-elements output target,
unless they also provide the dist output target as well

GitHub Issue Number: N/A

What is the new behavior?

Commit 1

this commit introduces a new experimental flag,
generateTypeDeclarations on the dist-custom-elements output target.
when enabled, it allows a user to generate type declaration files for
their stencil project (that uses dist-custom-elements) without also
specifying the dist output target.

this experimental flag generates types in the dist/types directory.
this destination is not configurable at this time to better understand
the needs of users for generating these types

Commit 2

this commit adds jsdoc, clarifying comments, and minor type updates to
the codebase. this work was completed while working through adding
generateTypeDeclarations to the dist-custom-elements output target,
but was split into a separate commit as to not detract from the work
being completed.

Does this introduce a breaking change?

  • Yes
  • No

Testing

Unit tests have been added to ensure that validating a dist-custom-elements output target returns the expected result. For these tests, I followed patterns existing in similar tests. While it may not be how I'd personally write them, i think maintaining consistency is more important in this matter, especially as we try to add new tests like these

Next, I generated a stencil project to serve as the foundation for a few manual tests (tested with Stencil v2.14.1):
npm init stencil component test-types.

By default, the Stencil CLI generates a Stencil config with the following:

{
  outputTargets: [
      {
        type: 'dist',
        esmLoaderPath: '../loader',
      },
      {
        type: 'dist-custom-elements',
      }
  ]
}

I ran npm run build - then took this branch and installed it on said project and ran npm run build again (without modifying my stencil config). I diffed the dist directory from both builds, and saw minimal differences (stencil versions and one typo fix):
Screen Shot 2022-03-08 at 2 17 22 PM.

Then, I modified my Stencil config for dist-custom-elements and removed dist:

{
  outputTargets: [
-     {
-       type: 'dist',
-       esmLoaderPath: '../loader',
-     },
      {
        type: 'dist-custom-elements',
+       generateTypeDeclarations: true,
      }
  ]
}

I then diff'ed this against the previous dist generated, and verified that the custom-elements contents were found in both dist directories, and unnecessary files were not in my newly generated dist:

Screen Shot 2022-03-08 at 2 18 51 PM

Finally, I modified my stencil config one last time to specify a custom output directory for dist-custom-elements. One more diff shows that the paths are set correctly for this build:

{
  outputTargets: [
      {
        type: 'dist-custom-elements',
        generateTypeDeclarations: true,
+       dir: 'custom-elements-specified-dir'
      }
  ]
}

Screen Shot 2022-03-08 at 2 22 59 PM

Other information

Documentation PR: ionic-team/stencil-site#845

this commit introduces a new experimental flag,
`generateTypeDeclarations` on the `dist-custom-elements` output target.
when enabled, it allows a user to generate type declaration files for
their stencil project (that uses `dist-custom-elements`) without also
specifying the `dist` output target.

this experimental flag generates types in the `dist/types` directory.
this destination is not configurable at this time to better understand
the needs of users for generating these types

STENCIL-329: Generate types for dist-custom-elements output target
this commit adds jsdoc, clarifying comments, and minor type updates to
the codebase. this work was completed while working through adding
`generateTypeDeclarations` to the `dist-custom-elements` output target,
but was split into a separate commit as to not detract from the work
being completed.

STENCIL-329: Generate types for dist-custom-elements output target
@rwaskiewicz rwaskiewicz changed the title Rwaskiewicz/dist ce types fix(types): generate types for dist-custom-elements Mar 8, 2022
@rwaskiewicz rwaskiewicz marked this pull request as ready for review March 9, 2022 13:01
@rwaskiewicz rwaskiewicz requested a review from a team March 9, 2022 13:01
@rwaskiewicz rwaskiewicz merged commit 04fb830 into main Mar 16, 2022
@rwaskiewicz rwaskiewicz deleted the rwaskiewicz/dist-ce-types branch March 16, 2022 19:39
rwaskiewicz added a commit that referenced this pull request Mar 24, 2022
this commit introduces a new experimental flag,
`generateTypeDeclarations` on the `dist-custom-elements` output target.
when enabled, it allows a user to generate type declaration files for
their stencil project (that uses `dist-custom-elements`) without also
specifying the `dist` output target.

this experimental flag generates types in the `dist/types` directory.
this destination is not configurable at this time to better understand
the needs of users for generating these types

this commit also adds jsdoc, clarifying comments, and minor type updates 
to the codebase.
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.

2 participants