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-24170] LiveView breaks relative path usage in require #105

Merged
merged 4 commits into from Sep 6, 2017

Conversation

feons
Copy link
Contributor

@feons feons commented Aug 9, 2017

@kolipakakondal
Copy link
Contributor

@feons - Since you've made changes in liveview.js file, do you see the need to change the same in
liveview.min.js? I remembered we were doing this for earlier PR's.

@kolipakakondal
Copy link
Contributor

I think we also need to merge the last 4 commits from the Chris on liveview repo. @feons -confirm?

Commits on Aug 11, 2017
@sgtcoolguy
Move to our new eslint standard, update to use let/const (#104) …
sgtcoolguy committed on GitHub 7 days ago
1 8b3f818
Commits on Aug 4, 2017
@sgtcoolguy
Fix up jscs warnings
sgtcoolguy committed 14 days ago
Verified 7b9a5a9
@sgtcoolguy
Fix up jshint warnings
sgtcoolguy committed 14 days ago
Verified 2f0827c
@sgtcoolguy
Add Jenkinsfile for build. Use grunt to build/test project instead of… …
sgtcoolguy committed 14 days ago
Verified 8151812

@feons
Copy link
Contributor Author

feons commented Aug 18, 2017

@kolipakakondal, is liveview.min.js used by studio? Also, Chris' commits had been merged to master awhile ago, do you see otherwise?

@kolipakakondal
Copy link
Contributor

@feons -

liveview.min.js used by studio?

No, I was thinking it would have used during the liveview.

Chris' commits had been merged to master awhile ago, do you see otherwise?

Since Chris changes are not merged into the studio so far, I've to merge those before I merge this PR changes.

@kolipakakondal
Copy link
Contributor

@feons - I'm facing below error while I'm verifying this PR.Took the latest repo changes and applied your PR on top of it. I'm having node 6.0, CLI 6.2.3 and SDK 6.2.0.

[ERROR] : Script Error {
[ERROR] : column = 23;
[ERROR] : line = 28;
[ERROR] : message = "undefined is not a function (evaluating 'Object.setPrototypeOf(Process.prototype, Emitter.prototype)')";
[ERROR] : sourceURL = "file:///Users/kkolipaka/Library/Developer/CoreSimulator/Devices/8EDBB933-4D8F-4F7E-852D-1F786E1FC163/data/Containers/Bundle/Application/D665C336-C1FA-44EA-833E-0CD47CC4D498/defaultalloyapp.app/app.js";
[ERROR] : stack = "file:///Users/kkolipaka/Library/Developer/CoreSimulator/Devices/8EDBB933-4D8F-4F7E-852D-1F786E1FC163/data/Containers/Bundle/Application/D665C336-C1FA-44EA-833E-0CD47CC4D498/defaultalloyapp.app/app.js:28:23\nglobal code@file:///Users/kkolipaka/Library/Developer/CoreSimulator/Devices/8EDBB933-4D8F-4F7E-852D-1F786E1FC163/data/Containers/Bundle/Application/D665C336-C1FA-44EA-833E-0CD47CC4D498/defaultalloyapp.app/app.js:717:3";
[ERROR] : }

@feons
Copy link
Contributor Author

feons commented Aug 21, 2017

@kolipakakondal, @sgtcoolguy, looks like liveview is broken with the latest changes on master, this PR has nothing to do with the error.

@feons
Copy link
Contributor Author

feons commented Aug 22, 2017

please revisit this after #107 is merged.

@feons
Copy link
Contributor Author

feons commented Aug 24, 2017

@kolipakakondal, #107 is merged to master, could you please test again.Thanks!

@kolipakakondal
Copy link
Contributor

Thanks @feons - It's working now.

@kolipakakondal
Copy link
Contributor

@feons - Did a rebase to get the latest master changes from #107

Copy link

@ssekhri ssekhri left a comment

Choose a reason for hiding this comment

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

@feons - The fix does not work on Android. Still get both the issues on Android. It works fine on iOS.
Tested from studio after the merge of liveview 1.2.1 into studio.

@feons
Copy link
Contributor Author

feons commented Sep 5, 2017

@ssekhri, this is android output, looks working to me.

 [LiveView]  Client connected
[INFO]  *** myMod.js ***
[INFO]  require_absolute_path: [object Object]
[ERROR] System: URI . has empty labels in the hostname. This is malformed and will not be acceptedin future Android releases.
[INFO]  *** myMod.js ***
[INFO]  require_relative_path: [object Object]
[ERROR] System: URI .. has empty labels in the hostname. This is malformed and will not be acceptedin future Android releases.
[ERROR] System: URI .. has empty labels in the hostname. This is malformed and will not be acceptedin future Android releases.
[ERROR] System: URI .. has empty labels in the hostname. This is malformed and will not be acceptedin future Android releases.
[ERROR] System: URI .. has empty labels in the hostname. This is malformed and will not be acceptedin future Android releases.
[INFO]  *** myMod.js ***
[INFO]  require_relative_path ../myMod: [object Object]
[INFO]  test1.x: 1
[INFO]  test2.x: 2

@feons
Copy link
Contributor Author

feons commented Sep 5, 2017

Update to avoid URIs warnings. @ssekhri, are you using Android 8.0?

@feons feons merged commit 18b644c into tidev:master Sep 6, 2017
@ssekhri
Copy link

ssekhri commented Sep 6, 2017

The latest commit fixes the issue on Android for TIMOB-24170

@ssekhri
Copy link

ssekhri commented Sep 6, 2017

@feons there is issue with latest commit as well. It is for TIMOB-24811 on Android. Works fine on iOS. Here is the output for CLI when ran on Android emulator with liveview flag
`
[LiveView] version 1.2.1
[LiveView] Alloy project monitor started
[LiveView] File Server Started on 0.0.0.0:8324
[LiveView] Event Server Started on 0.0.0.0:8323
[INFO] App successfully installed
[INFO] Starting app: a.a.a/.AlloyprojActivity
[INFO] Application pid: 6686
[INFO] Project built successfully in 36s 903ms

-- Start application log -----------------------------------------------------
[INFO] TiApplication: (main) [1,1] checkpoint, app created.
[INFO] TiApplication: (main) [56,57] Titanium 6.1.2 (2017/07/27 16:09 undefined)
[INFO] MultiDex: VM with version 2.1.0 has multidex support
[INFO] MultiDex: install
[INFO] MultiDex: VM has multidex support, MultiDex support library is disabled.
[INFO] TiApplication: (main) [128,185] Titanium Javascript runtime: v8
[INFO] TiRootActivity: (main) [0,0] checkpoint, on root activity create, savedInstanceState: null
[LiveView] Client connected
[ERROR] TiHTTPClient: (TiHttpClient-10) [1,1009] HTTP Error (java.io.IOException): 404 : Not Found
[ERROR] TiHTTPClient: java.io.IOException: 404 : Not Found
[ERROR] TiHTTPClient: at ti.modules.titanium.network.TiHTTPClient$ClientRunnable.run(TiHTTPClient.java:1283)
[ERROR] TiHTTPClient: at java.lang.Thread.run(Thread.java:818)
[INFO] test1.x: 1
[INFO] test2.x: 1
`
I will share the alloy project with you that I used to test this.

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

4 participants