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(registry): 🐛 clear registry at start #37

Merged
merged 6 commits into from
Sep 5, 2024

Conversation

fb-sean
Copy link
Contributor

@fb-sean fb-sean commented Sep 4, 2024

Why?

Simple, when you drop the error, the registry don't get cleared. Which can lead into followup issues and since the registry get cleared at the end anyway it dosnt really matter.

Copy link

changeset-bot bot commented Sep 4, 2024

🦋 Changeset detected

Latest commit: 8d2b1f8

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@heatsrc/vue-declassified Patch
@heatsrc/vuedc Patch
@vuedc-internal/playground Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@jaredmcateer
Copy link
Contributor

Nice catch, thanks!

Co-authored-by: Jared McAteer <[email protected]>
@jaredmcateer
Copy link
Contributor

Ah so this introduces an issue, the convertMixin function calls setIsMixin() which sets a registry property then calls convertScript which now immediately clears the registry.

I think maybe a better solution is wrapping the convertScript in a try/finally block and calling resetRegistry in the finally section.

@fb-sean
Copy link
Contributor Author

fb-sean commented Sep 5, 2024

What do you think about this way?

Co-authored-by: Jared McAteer <[email protected]>
@jaredmcateer
Copy link
Contributor

jaredmcateer commented Sep 5, 2024

Okay looks good, one last thing, can you please add a changeset?

From project root run,

pnpm changeset

And follow the prompts

  • Choose just the changed package (ie, @heatsrc/vue-declassified)
  • No selections for major or minor just press enter to create a patch changeset
  • Add a descriptive summary (e.g., Reset registry when stopping on collisions)

@fb-sean
Copy link
Contributor Author

fb-sean commented Sep 5, 2024

sure

@jaredmcateer jaredmcateer merged commit 332ff52 into heatsrc:main Sep 5, 2024
0 of 2 checks passed
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