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

[TIMOB-26179] Check for property existence using hasProperty #10150

Merged
merged 3 commits into from Jul 12, 2018

Conversation

janvennemann
Copy link
Contributor

JIRA: https://jira.appcelerator.org/browse/TIMOB-26179

Optional Description:
Implements the hasProperty callback to properly check for property existence in our proxy objects. Without this callback getProperty is which results in property checks always returning true.

@hansemannn Not sure if this implies a breaking change or can just be considered as a re-definition of previously undefined behavior.

@build
Copy link
Contributor

build commented Jul 3, 2018

Fails
🚫

Tests have failed, see below for more information.

Messages
📖

💾 Here's the generated SDK zipfile.

Tests:

Classname Name Time Error
ios.Titanium.UI.WebView userAgent 10.009 file:///Users/build/Librar

Generated by 🚫 dangerJS

@hansemannn
Copy link
Collaborator

LGTM. But please add a unit-test for it since it should be somehow testable from our proxy implementation. Also, do you have a practical use case for this that is worth mentioning?

NSString *name = (NSString *)TiStringCopyCFString(kCFAllocatorDefault, propertyName);
[name autorelease];
id result = [o valueForKey:name];
if (result != nil) {
Copy link
Contributor

@sgtcoolguy sgtcoolguy Jul 5, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

collapse the last few lines down to:

return result != nil;

@sgtcoolguy
Copy link
Contributor

I think it'd be pretty easy to add a quick unit test that generated a proxy and then just tested for a non-existent and existing own property. I believe iOS always returned true before?

var label = Ti.UI.createLabel();
should(label.hasOwnProperty('madeup')).eql(false);
should(label.hasOwnProperty('text')).eql(true);
label.madeup = 123;
should(label.hasOwnProperty('madeup')).eql(true);

@sgtcoolguy sgtcoolguy changed the title [TIMOB-26179] Check for property exitence using hasProperty [TIMOB-26179] Check for property existence using hasProperty Jul 11, 2018
@sgtcoolguy sgtcoolguy added this to the 7.4.0 milestone Jul 12, 2018
@sgtcoolguy sgtcoolguy merged commit d32249b into tidev:master Jul 12, 2018
@hansemannn hansemannn modified the milestones: 7.4.0, 7.5.0 Aug 24, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants