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

feat(ios): support circular require references #11693

Merged
merged 21 commits into from Jan 13, 2021

Conversation

sgtcoolguy
Copy link
Contributor

@sgtcoolguy sgtcoolguy commented May 7, 2020

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

Description:
This is a PR to unify our require implementation between iOS and Android. Android had a mostly JS-based implementation based off Node.js. The native layer uses bindings for native/core modules and loading those. But the bulk of loading JS code is actually done in JS. On iOS, the entire flow was native.

This PR has been carefully rebased into meaningful commits to help Code Reviews (look at the individual commits, the full PR is overwhelming!).

The commits generally go as follows:

  • Copy the Node.js require implementation into common and hijack iOS require to be replaced by it during ti.main.js load (and then add a test to verify it fixes the circular require issue in an app)
  • Rip out the native implementation of require on iOS and do a refactoring of the code to make the underlying bindings cleaner.
  • Create a new separate bundle in common for ti.kernel.js - equivalent to Android's kroll.js. This consists of the very core kernel JS scripts to set up the JS environment before ti.main.js is loaded.
  • Refactor iOS startup to more closely match Android by executing ti.kernel.js bootstrap function - the most important/relevant thing it defines is Module - the class that does require. Then when we're asked to run ti.main.js or a service entry point, we run it though Module.runModule().
  • Then refactor Android to use the common/ti.kernel.js in place of it's old kroll.js. The extensions to titanium APIs are moved out into the common/ti.internal/extensions/ti folder to occur during ti.main.js.

@build
Copy link
Contributor

build commented May 7, 2020

Warnings
⚠️

android/titanium/libv8-services.js#L54 - android/titanium/libv8-services.js line 54 – 'debugGenerateSnapshot' is defined but never used. (no-unused-vars)

Messages
📖 👍 Hey!, You deleted more code than you added. That's awesome!
📖

💾 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 13960 tests are passing.
(There are 899 skipped tests not included in that total)

Generated by 🚫 dangerJS against 1d69030

@sgtcoolguy
Copy link
Contributor Author

For some reason, this PR appears to make it so we cannot permanently alter Ti.Blob's prototype from JS to add the arrayBuffer method. I was able to implement the method natively after cherry-picking the Promise implementation from #10554 and defining arrayBuffer in Obj-C.

See:

@vijaysingh-axway
Copy link
Contributor

@sgtcoolguy While running the PR using Xcode, I am getting following error -
`Titanium[33147:305062] [ERROR] /ti.kernel.js:1214
fileIndex = JSON.parse(json);
^
SyntaxError: JSON Parse error: Unexpected identifier "undefined"
at parse ([native code])
at filenameExists (/ti.kernel.js:1214:32)
at loadAsFile (/ti.kernel.js:1085:31)
at loadAsFileOrDirectory (/ti.kernel.js:1017:36)
at require (/ti.kernel.js:828:49)
at Object. (file:///ti.main.js:14814:10)
at loadAsync (file:///ti.main.js:14760:13)
at Object. (file:///ti.main.js:14811:10)
at load (/ti.kernel.js:708:23)
at runModule (/ti.kernel.js:1258:17)

Foundation 0x0000000104dfeef0 _NSDescriptionWithLocaleFunc + 93
CoreFoundation 0x000000010adb489c __CFStringAppendFormatCore + 11852` 

@vijaysingh-axway
Copy link
Contributor

@sgtcoolguy While running the PR using Xcode, I am getting following error -
`Titanium[33147:305062] [ERROR] /ti.kernel.js:1214
fileIndex = JSON.parse(json);
^
SyntaxError: JSON Parse error: Unexpected identifier "undefined"
at parse ([native code])
at filenameExists (/ti.kernel.js:1214:32)
at loadAsFile (/ti.kernel.js:1085:31)
at loadAsFileOrDirectory (/ti.kernel.js:1017:36)
at require (/ti.kernel.js:828:49)
at Object. (file:///ti.main.js:14814:10)
at loadAsync (file:///ti.main.js:14760:13)
at Object. (file:///ti.main.js:14811:10)
at load (/ti.kernel.js:708:23)
at runModule (/ti.kernel.js:1258:17)

Foundation 0x0000000104dfeef0 _NSDescriptionWithLocaleFunc + 93
CoreFoundation 0x000000010adb489c __CFStringAppendFormatCore + 11852` 

After new commits, it is working fine. Thanks!

- retains native require but overrides during app startup to match Node.js/Android
- require logic is almost entirely in JS now
- expose some extra native methods to implement similarly to Android
- fix js core "binding" hack to work on ios in same way as android
* move Script out to separate Module
* break out kroll and assets globals to separate modules
* introduce Module protocol to unify legacy/new modules
* add APIs to convert new style proxies to JSValue*
* fix conversion of new style modules via kroll.binding()
* split ti.main and ti.kernel bundling
* begin effort to unify the startup/code between platforms
* use split bundling methods to produce proper parts in proper build stages
* don't include ti.kernel.js for Android, as it gets baked into C code
* removes the need to pass around context objects everywhere
* remove the now-unused AndroidWrapper stuff
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

6 participants