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

hyperscript: handles shared empty attrs, fixes #2821 #2822

Merged
merged 1 commit into from
Feb 23, 2023

Conversation

kfule
Copy link
Contributor

@kfule kfule commented Feb 15, 2023

Whenever there are selector-derived attrs, the attrs object will be regenerated and not shared.

Description

When selector-derived attrs exist, the attrs object is regenerated. However, if the attrs object is empty, the attrs object is not regenerated, and the selector-derived attrs is unintentionally shared.
In this pr, the condition !isEmpty(attrs) was removed so that even if the attrs object is empty, the attrs object is regenerated if there are selector-derived attrs.

Motivation and Context

One tiny bug #2821 has been fixed, and the code outlook is a bit better.

How Has This Been Tested?

Added test and ran npm run build.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation change

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have updated docs/changelog.md

@dead-claudia dead-claudia merged commit 88da9e5 into MithrilJS:next Feb 23, 2023
@JAForbes JAForbes mentioned this pull request Feb 23, 2023
@kfule kfule deleted the shared-empty-attrs branch March 25, 2023 11:53
This was referenced Sep 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants