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

(6_1_X) [TIMOB-24932] Android Hybrid modules: cannot instantiate a proxy in a js file that is packaged in the same module #9206

Merged
merged 3 commits into from Jul 11, 2017

Conversation

sgtcoolguy
Copy link
Contributor

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

Description:
Calling require('module.id') from inside a JS file in that module's assets folder would fail (specifically this is when you're creating "hybrid" native modules for Android that combine Java proxies and JS code). See the ticket for steps to reproduce.

The fix here was to avoid clobbering the native half of the hybrid module in our cache object. Before we'd insert the CommonJS half of the hybrid API into our require cache over top the native module. We need to keep just the native half and recreate the combination of both when required from different files since apparently it's context specific (and so the combined wrapper is cached per-module).

…in a js file that is packaged in the same module
Copy link
Contributor

@jquick-axway jquick-axway left a comment

Choose a reason for hiding this comment

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

CR: Pass

@ewanharris ewanharris self-requested a review July 10, 2017 15:45
Copy link
Collaborator

@ewanharris ewanharris left a comment

Choose a reason for hiding this comment

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

FR Pass!

Node.js 6.11.0
OSX 10.12.5
Appc CLI 6.2.2
Appc NPM 4.2.9
Appcelerator Studio 4.9.0.201705302345
Android Device 7.1.1 OnePlus 3
FR Passed

Demo in ticket works as expected now and calling native functions from a packaged js file works. Exercised functionality such as requiring other modules and it also worked as expected

@ewanharris ewanharris merged commit 4939098 into tidev:6_1_X Jul 11, 2017
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

3 participants