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

Release/2.3.1 #58

Open
wants to merge 12 commits into
base: master
Choose a base branch
from
Open

Release/2.3.1 #58

wants to merge 12 commits into from

Conversation

Phanabani
Copy link
Collaborator

@Phanabani Phanabani commented Nov 10, 2022

2.3.1

Fix

  • keyPath, the string[] argument in many callback functions, is now frozen, so mistakenly trying to mutate it will throw an error
  • Fixed dev_app's read-only function checkbox throwing an error
  • Cleaned up checkbox logic in dev_app (no longer uses unnecessary refs)

Docs

  • Add info about how refs must be attachable to inputElement

@Phanabani Phanabani added this to the 2.3.1 milestone Nov 10, 2022
@Phanabani Phanabani self-assigned this Nov 10, 2022
@Phanabani
Copy link
Collaborator Author

@oxyno-zeta I hope you're not bothered by me requesting reviews for these pull requests! Let me know if there's anything I can do to make things easier for you.

I've already tested this myself in the dev app, but wanted to make sure you're okay with my changes before merging into master. And you also have npm publish rights! 😄

@oxyno-zeta
Copy link
Owner

oxyno-zeta commented Nov 10, 2022

Hello @Phanabani ,

Thanks a lot for developing on this project. Reviewing your pull requests is always a pleasure.

One quick question: why do you consider the freeze as a fix and not a feature? I'm not telling you to change because can be both for me, just wondering.

Will read as soon as possible. May be next week if it isn't today (holidays tomorrow ;) ).

Oxyno-zeta

@Phanabani
Copy link
Collaborator Author

Hi @oxyno-zeta!

I'm happy that you appreciate my work! It means a lot to me.

I considered the freeze as a fix and not a feature because the user mutating keyPath is unintended behavior and produces bad internal side effects since we retain and reuse a reference to the array. I think I would consider it a feature if the side effects weren't involved.

I hope you have a good holiday weekend! 😄

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