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

Expose template registry and drop rollup-plugin-ts #194

Merged
merged 5 commits into from
Sep 5, 2023

Conversation

vscav
Copy link
Member

@vscav vscav commented Sep 1, 2023

Can be reviewed commit by commit.

Context

This PR aims at improving the TypeScript experience and aligning with the last best practice.

How

First, we now expose the template-registry of the Lottie component. It allows proper template typing in consuming applications using TypeScript with a Glint setup (see how-to from Glint).

Then, we propose to move off rollup-plugin-ts, following the latest guidelines from addon-blueprint (see PR for details). Note that in our case, using rollup-plugin-ts, it became impossible to build the addon locally (and so impossible to test it locally in consuming applications). Now the type declarations will be outputted in a separate declarations folder inside the build folder.

Examples

Wrong argument

wrong-arg

Wrong type

wrong-value

Tasks

  • Upgrade glint dependencies
  • Export component template-registry
  • Move off rollup-plugin-ts
  • Add script to run linter for Glint
  • Add documentation for users wanting to use the addon in an Ember application set up with TS
  • Use yalc to test the addon (build and usage) locally in both JS and TS ember applications

@vscav vscav added the enhancement New feature or request label Sep 1, 2023
@vscav vscav self-assigned this Sep 1, 2023
@vscav vscav marked this pull request as ready for review September 1, 2023 16:33
@vscav vscav changed the title Expose registry and drop rollup-plugin-ts Expose template registry and drop rollup-plugin-ts Sep 1, 2023
Copy link
Contributor

@SkoebaSteve SkoebaSteve left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, this is a welcome improvement!

@vscav vscav merged commit 402b53d into main Sep 5, 2023
10 checks passed
@vscav vscav mentioned this pull request Sep 26, 2023
@dbendaou dbendaou deleted the drop-rollup-plugin-ts branch December 27, 2023 10:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants