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

fix(android): module logDeprecation() linker error #12259

Merged
merged 5 commits into from Nov 23, 2020

Conversation

jquick-axway
Copy link
Contributor

@jquick-axway jquick-axway commented Nov 10, 2020

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

Notes:

  • This fixes a regression introduced by 9.1.0.
  • If a module was built with Titanium 9.1.0 or higher and it logs a getter/setter property method deprecation warning, then it will crash an on app startup if app was built with Titanium 9.0.0 - 9.0.3.
  • Fails to link with logDeprecation() function added to 9.1.0.
  • Solved by reverting to LOGW() for module builds only.
  • Titanium SDK builds will still use logDeprecation() function which provides a stack trace.

SDK Test:

  1. Build and run the below on Android.
  2. Verify it logs the following warnings on app startup.
[WARN]  Proxy: Setter method deprecated, please use "obj.text = value;" or "obj['text'] = value;" instead.
[WARN]  Proxy: label.setText("Hello World"),
[WARN]  Proxy:       ^
[WARN]  Proxy:     at (/LabelTest.js:11:7)
[WARN]  Proxy: Getter method deprecated, please use "obj.text;" or "obj['text'];" instead.
[WARN]  Proxy: label.getText(),
[WARN]  Proxy:       ^
[WARN]  Proxy:     at (/LabelTest.js:12:7)

app.js

var window = Ti.UI.createWindow();
var label = Ti.UI.createLabel();
label.setText("Hello World");  // <- Deprecated
label.getText();  // <- Deprecated
window.add(label);
window.open();

Module Test:

  1. Download the ti.identity module source.
  2. CD to ./titanium-identity/android.
  3. Make sure to appc ti sdk select <Version> this PR's SDK version.
  4. Enter: `appc run -p android --build-only
  5. Create a Titanium project with below "tiapp.xml" and "app.js" files. (Must build with 9.0.3.)
  6. Copy ./titanium-identity/android/dist/ti.identity-android-3.0.3.zip
  7. Paste to Titanium app project's root folder.
  8. Unzip module. (Should now have a "module" folder.)
  9. Build and run on Android.
  10. Verify app does not crash on app startup.
  11. Verify the following deprecation warnings are logged.
[WARN]  TitaniumIdentityModule: Setter method deprecated, please use "obj.authenticationPolicy = val;" or "obj['authenticationPolicy'] = val;" instead.
[WARN]  TitaniumIdentityModule: Getter method deprecated, please use "obj.authenticationPolicy;" or "obj['authenticationPolicy'];" instead.

tiapp.xml

<ti:app>
	<modules>
		<module>ti.identity</module>
	</modules>
	<sdk-version>9.0.3.GA</sdk-version>
</ti:app>

app.js

var tiIdentity = require('ti.identity');
tiIdentity.setAuthenticationPolicy(tiIdentity.AUTHENTICATION_POLICY_BIOMETRICS);
console.log("@@@ ti.identity policy: " + tiIdentity.getAuthenticationPolicy());

var window = Ti.UI.createWindow();
window.add(Ti.UI.createLabel({
	text: "Device can authenticate: " + tiIdentity.isSupported(),
}));
window.open();

@build
Copy link
Contributor

build commented Nov 10, 2020

Messages
📖

💾 Here's the generated SDK zipfile.

📖 ✊ The commits in this PR match our conventions! Feel free to Rebase and Merge this PR when ready.
📖

✅ All tests are passing
Nice one! All 12601 tests are passing.
(There are 902 skipped tests not included in that total)

Generated by 🚫 dangerJS against defb536

Copy link
Contributor

@sgtcoolguy sgtcoolguy left a comment

Choose a reason for hiding this comment

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

LGTM

@sgtcoolguy sgtcoolguy modified the milestones: 9.3.0, 10.0.0 Nov 20, 2020
@lokeshchdhry
Copy link
Contributor

FR Passed.

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

5 participants