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

Stronger typing for Archetype #102

Merged
merged 2 commits into from Aug 2, 2022
Merged

Stronger typing for Archetype #102

merged 2 commits into from Aug 2, 2022

Conversation

benwest
Copy link
Contributor

@benwest benwest commented Aug 1, 2022

Hello, I'm playing around with this library for a game project and enjoying it so far ✌️. I ran into a few hiccups with the typings for Archetype and I think these changes make sense - let me know if I misunderstood something.

– Entities emitted by onEntityAdded can have their type narrowed by the query
– But I think those in onEntityRemoved can't, because they may have been removed for no longer matching the query?
first can return null, but the inferred type didn't know that
– and I also added an assertion to entityIsArchetype so Archetype.indexEntity no longer needs to use any.

@changeset-bot
Copy link

changeset-bot bot commented Aug 1, 2022

🦋 Changeset detected

Latest commit: 93c2edf

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

This PR includes changesets to release 1 package
Name Type
miniplex 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

@hmans
Copy link
Owner

hmans commented Aug 2, 2022

Hi, thanks for the PR! I'm going to play around with it today, but I think the changes are sane. May I ask you to exclude the changes to yarn.lock and the addition of package-lock.json from this PR? Thanks!

Screen Shot 2022-08-02 at 05 19 47

@benwest
Copy link
Contributor Author

benwest commented Aug 2, 2022

yes!

@hmans
Copy link
Owner

hmans commented Aug 2, 2022

Hi, can you change this PR so it does not delete package.json?

@benwest
Copy link
Contributor Author

benwest commented Aug 2, 2022

Oof 😮‍💨 - wrong file! sorry, that's amended and squashed.

@hmans
Copy link
Owner

hmans commented Aug 2, 2022

Thank you! \o/

@hmans hmans merged commit 1cee12c into hmans:main Aug 2, 2022
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.

None yet

2 participants