Skip to content

animint.js: remove unnecessary hasOwnProperty selector checks #284

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.

Sign up for GitHub

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

Jump to bottom
Open
harshiittttt wants to merge 6 commits into
base: master
Choose a base branch
from
Open

animint.js: remove unnecessary hasOwnProperty selector checks #284

harshiittttt wants to merge 6 commits into from

Conversation

@harshiittttt
Copy link

@harshiittttt harshiittttt commented Dec 15, 2025

Fixes #278.

This PR removes unnecessary hasOwnProperty checks when accessing
Selectors in animint.js.

I verified the current source and fixed all existing occurrences
(~1570, ~1866, ~2087). Two additional line references mentioned in
the issue (~2766, ~2797) do not exist in the current animint.js
source, which ends at ~2754 lines.

@harshiittttt harshiittttt mentioned this pull request Dec 15, 2025
Open
tdhock
tdhock requested changes Dec 15, 2025
View reviewed changes
@tdhock
Copy link
Collaborator

tdhock commented Dec 15, 2025

did you read the contribution guidelines? https://github.com/animint/animint2?tab=contributing-ov-file#avoiddeclare-ai-code-generation
do you agree to avoid AI code generation tools like Copilot?

@harshiittttt
Copy link
Author

harshiittttt commented Dec 15, 2025

yes, i have read the contribution guidelines and agree to follow the same. I have removed the if check . please let me know about any further changes

@harshiittttt
Copy link
Author

harshiittttt commented Dec 16, 2025

hi, @tdhock , just saw the tests have failed, do you have any suggestion on how to fix the issue or any technical documentation i need to go thorough to fix this issue.

@tdhock
Copy link
Collaborator

tdhock commented Dec 16, 2025

thanks! it is normal for the gh pages tests to fail from forks https://github.com/animint/animint2?tab=contributing-ov-file#prs-from-forks-versus-branches-in-this-repo

can you please add your name as a contributor to DESCRIPTION, and increase version number there, and add a NEWS item?

@harshiittttt
Copy link
Author

harshiittttt commented Dec 16, 2025

done adding , PR ready for review , please let me know about any further changes.

tdhock
tdhock requested changes Dec 16, 2025
View reviewed changes
@harshiittttt
Copy link
Author

harshiittttt commented Dec 16, 2025

Ready for review

tdhock
tdhock reviewed Dec 16, 2025
View reviewed changes
@harshiittttt
Copy link
Author

harshiittttt commented Dec 17, 2025

Hi @tdhock , pr ready for review.
would love to work on other beginner friendly issues as well.

@tdhock
Copy link
Collaborator

tdhock commented Dec 17, 2025

there are a lot of failing tests.
can you please debug and fix?

@harshiittttt
Copy link
Author

harshiittttt commented Dec 17, 2025

Thanks for the feedback ,I'll start debugging for the failing tests.

@ANAMASGARD @harshiittttt
fix: Remove unnecessary hasOwnProperty checks in animint.js
1e70418 
Removed all Selectors.hasOwnProperty checks and added proper defensive
checks to prevent undefined selector access errors.

Changes:
- Line ~1570: Added if-check before accessing selector duration
- Line ~1866: Added if-check for transition duration
- Line ~2084: Added early return if selector doesn't exist
- Line ~2534-2536: Removed hasOwnProperty for clickSelects and legend checks

All hasOwnProperty checks removed while maintaining safety through
defensive programming.

Fixes animint#278

Co-authored-by: Harshit Gangwar <bethebest100times@gmail.com>
@ANAMASGARD ANAMASGARD requested a review from tdhock January 3, 2026 14:03
@ANAMASGARD
Copy link
Contributor

ANAMASGARD commented Jan 3, 2026 •
edited
Loading

Sir @tdhock

  • Removed hasOwnProperty checks in animint.js
  • Added defensive checks to prevent undefined selector access.
  • The 2 failing tests (test-compiler-ghpages.R) are GitHub authentication
    errors, not related to the hasOwnProperty fixes. As mentioned in the contribution
    guide, gh pages tests fail from forks.

Sir please review and give your feedback , Thanks .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tdhock tdhock Awaiting requested review from tdhock

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

animint.js Selectors does not need hasOwnProperty

3 participants

@harshiittttt @tdhock @ANAMASGARD