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-20469 support building for both Titanium class and Alloy #27

Merged
merged 2 commits into from May 10, 2016
Merged

TIMOB-20469 support building for both Titanium class and Alloy #27

merged 2 commits into from May 10, 2016

Conversation

jhaynie
Copy link
Contributor

@jhaynie jhaynie commented Apr 28, 2016

This PR needs to be well tested before merging and as the chance of breaking things since it's pretty invasive.

To test, you need to first build an SDK from master ensuring this commit.

You need to test both Android and iOS and both Titanium Classic and Titanium Alloy projects separately. We also need to test this with the hyperloop-examples project with these changes.

The other test we need to do is verify that this works both on simulator and device. For a device build, we need to ensure that no source code (objective-C) for iOS or (Java) for Android is in the final package.

I've tried to make minimal changes to make this work. The biggest changes were in iOS. For iOS, I switched to doing the hyperloop JS work on each file instead of all at front -- that way I didn't have to worry about making changes to Resources intermediate files -- instead just making changes to the final destination.

Since this is pretty complicated, I think we should have both @hansemannn and @cheekiatng both review this before merging.

@hansemannn
Copy link
Contributor

hansemannn commented Apr 29, 2016

Comments which I also posted in the flow:

@jhaynie tested the PR using the hyperloop-examples app: Works great on sim, fails during device-build process:

[DEBUG] No change, skipping /Users/hans/Documents/Apps/hyperloop-examples/build/iphone/assets/log_js
[DEBUG] Copying and minifying /Users/hans/Documents/Apps/hyperloop-examples/Resources/iphone/subclasses/chartdelegate.js => /Users/hans/Documents/Apps/hyperloop-examples/build/iphone/assets/subclasses/chartdelegate_js

// Debug logs from me (node.type and JSON.stringify(node))
UnaryExpression
{"type":"UnaryExpression","start":130,"end":132,"loc":{"start":{"line":1,"column":130},"end":{"line":1,"column":132}},"operator":"!","prefix":true,"argument":{"type":"Literal","start":131,"end":132,"loc":{"start":{"line":1,"column":131},"end":{"line":1,"column":132}},"value":0,"raw":"0"}}

Caught exception: JSParseError: not sure what to do with this node (undefined:1:130)

the problem is, that the expression instance: true inside delegate definitions is formed to instance: !0 which is super ugly on the first hand, but its not handled properly by the metabase generator (the node-type UnaryExpression is not switch-case-handled). Interesting: Changing true to 1 makes the build succeed, but the app doesn't work with "Cannot find Hyperloop" in the JS metabase wrappers.

I hope that helps. the only device-related thing i can think of is 9ecc22f.

Note: This build-failture did not happen with latest master revision.

EDIT: Also tested it with a classic project. The build succeeds (probably because there are no delegates placed in subclasses). Anyway, it fails using this sample. Log:

[ERROR] Script Error {
[ERROR]     column = 26;
[ERROR]     line = 7;
[ERROR]     message = "Can't find variable: Hyperloop";
[ERROR]     sourceURL = "file:///Users/hans/Library/Developer/CoreSimulator/Devices/765A790D-A9C5-4C7C-B3F7-15D024664D22/data/Containers/Bundle/Application/55F1A43D-6479-4F3D-9B6C-6E41AF916CCD/hl.app/hyperloop/uikit/uiview.js";
[ERROR] } 
[ERROR] Script Error Module "hyperloop/uikit/uiview" failed to leave a valid exports object
[ERROR] ErrorController is up. ABORTING showing of modal controller

@hansemannn
Copy link
Contributor

hansemannn commented May 4, 2016

Alloy build succeeds now, but still getting the above error happening with classic projects as well (Can't find variable: Hyperloop).

@cheekiatng
Copy link
Contributor

PR actually works well for me. Having QE to help check one more time before merging.

@hansemannn
Copy link
Contributor

Summing up:

  • It does work when the app is registered, it does not work with an 1111-... guid
  • Kiat was able to reproduce the issue with the example project provided

You can decide if we should focus on that issue on a different ticket, but I don't feel like we merge too fast now.

@cheekiatng
Copy link
Contributor

As Hans have clarified. It only errors if I used GUI 111-11-1111...

@hansemannn
Copy link
Contributor

hansemannn commented May 10, 2016

CR and FT passed, PR approved! Thanks Jeff! 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants