[TIMOB-25748] iOS: Remove TiCore in favor of built-in JavaScriptCore #9798

hansemannn · 2018-02-04T10:19:28Z

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

⚠️ Do not merge, yet! We need to wait until master is 8.0.0.

no tests as there are no new API's, but all existing test-suites should pass.

…libTiCore

build · 2018-02-04T10:22:31Z

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

Generated by 🚫 dangerJS

sgtcoolguy · 2018-02-06T15:43:22Z

build/ios.js

 IOS.prototype.build = function (next) {
-	this.fetchLibTiCore(next);
+	// no-op (used to fetch TiCore in the past)


Clearly not related to this PR, but this jogs my memory. Weren't we going to pre-build libraries for iOS like we do for the other platforms at some point? Do we have JIRA tickets/PRs to track that?

I remember you had an angry TODO comment somewhere that states that we should just place libraries inside the repo instead of fetching them manually. In case you mean that, it should be obsolete now that we remove TiCore. Other libraries like libAPSHTTPClient and libAPSAnalytics are prepackaged, just like Hyperloop should / will be in the future.

sgtcoolguy · 2018-02-06T15:54:08Z

build/ios.js

@@ -80,7 +54,6 @@ IOS.prototype.package = function (packager, next) {
 	const DEST_IOS = path.join(packager.zipSDKDir, 'iphone');

 	async.parallel([
-		this.fetchLibTiCore.bind(this),


Also need to remove the step below where we try to copy iphone/headers/JavaScriptCore since there's no files there anymore (that's why the build is failing right now, lines 62-64 in your modified file) - and the 'headers' entry in the array of folders/files we try to copy (line 66 in your modified file)

sgtcoolguy · 2018-02-06T15:55:34Z

build/ios.js

@@ -80,7 +54,6 @@ IOS.prototype.package = function (packager, next) {
 	const DEST_IOS = path.join(packager.zipSDKDir, 'iphone');

 	async.parallel([
-		this.fetchLibTiCore.bind(this),


Can we also remove libti_ios_debugger.a? libti_ios_profiler.a?

Those were to be used against TiCore and do not work with JSCore.

I've been told several times we can kill off our profiler because it's not useful and no one uses/knows about it. The debugger has been replace by using the official debugger APIs built into JSCore.

TiProfiler is dead. I think it was used in the past when Titanium wasn't fully compatible with Instruments, which works fine these days. I'll search related guides and docs to clarify that.

…ed code in iOS build script.

hansemannn · 2018-07-20T19:33:29Z

@sgtcoolguy Ready for re-review

build · 2018-07-20T19:47:01Z

	Messages
📖	👍 Hey!, You deleted more code than you added. That's awesome!
📖	💾 Here's the generated SDK zipfile.

Generated by 🚫 dangerJS

sgtcoolguy · 2018-07-25T14:58:18Z

iphone/cli/commands/_build.js

@@ -5838,10 +5799,6 @@ iOSBuilder.prototype.copyResources = function copyResources(next) {
 									transpile: this.transpile,
 									resourcesDir: this.xcodeAppDir
 								};
-								// generate our transpile target based on tijscore/jscore
-								if (this.useJSCore) {
-									analyzeOptions.targets = { ios: this.minSupportedIosSdk }; // if using jscore, target our min ios version


The block can't be removed entirely. That option setting needs to be moved up above then (because useJSCore is always true now). Without setting analyzeOptions.targets = { ios: this.minSupportedIosSdk }; we'll transpile everything down, not selectively based on the JS engine in iOS.

Dangerous issue, thanks for picking this up!

sgtcoolguy · 2018-07-25T15:04:14Z

package-lock.json

@@ -2204,7 +2204,7 @@
    "file-state-monitor": {
      "version": "1.0.0",
      "resolved": "https://registry.npmjs.org/file-state-monitor/-/file-state-monitor-1.0.0.tgz",
-      "integrity": "sha512-Tmn8VmqNW++Vyd0aMgNPR1F+56gp4GE9iY1rpM5X4YrN1yDxLVBfL2Q7qr6FCyoYyNiyOHCFidK1Mpl5t58BSA==",


I'd revert your changes to package-lock.json - it looks like it's using an older npm or something as it seems to have just replaced the integrity hashes with sha1 from the existing sha512 versions and toggled some other internal boolean flags - but changed no actual packages/versions used.

sgtcoolguy · 2018-07-25T15:26:33Z

iphone/headers/JavaScriptCore/TiBase.h

@@ -1,155 +0,0 @@
-/**


This file is referenced in the native module template. I assume that the removal of these headers will mean that native modules need to be rewritten to #import JavaScriptCore/JavaScriptCore.h> in place of #import #TiBase.h - and that any Ti* methods/types need to be JS* now too?

Do we want to retain these headers to keep the modules from having to do that?

Maybe modify them to include what TiToJS.h had before (basically just the the aliasing of Ti* to JS*)?

Or do we anticipate native modules needing to be rebuilt/modified anyways (with this or other changes)? If so we should bump the api version in the package.json to reflect that module's need to be rebuilt and are not backwards compatible.

So to follow on:

I was mistaken and thought TiBase.h was removed in this PR, but it is not.

However, The native module template uses TiModule.h, which tracing up the stack imports KrollObject.h which swapped from ToTiJS.h to the actual JavaScriptCore.h.

This means that any aliased Ti* types are now gone. These types were actually available to native modules (though I'm not sure how many actually used them directly). As a result, this is a breaking api change for modules, so the api version for iOS modules should be bumped: https://github.com/appcelerator/titanium_mobile/blob/master/package.json#L6

That would mean that native modules would need to be rebuilt/re-released. I'm not sure if they were going to need to be anyways as a result of this change...

Did we test that so far? I would reaaaally like to prevent a breaking module change wherever possible. One possible solution could be to leave the bridge file where it is (legacy), but add a deprecation warning for developers to migrate them. I am planning the same for the Swift module effort, which would basically have a "shelled" file that only imports existing API's. This could eventually work out. Should I try that in this PR, a different one or should we break the modules intentionally?

@sgtcoolguy I just tested with an existing app (that uses Ti.Facebook v3.8.0) and it works without recompiling the module. I think that unless we actively use TiCore symbols in modules (like TiValueRef), we are fine. And since those are supposed to be private API anyway, people should not be using it at all.

…tance of "${applicationId}" in AAR manifest. (tidev#10206) - Now replaces all ${applicationId} instances in AAR AndroidManfest.xml. - Note: This was a bug when building with module "titanium-firebase-core" via Titanium 7.3.0.

hansemannn added 3 commits February 4, 2018 10:39

Remove TiCore

8c20c0a

Remove Ti -> JS aliases, remove TiCore references in the CLI, unlink …

4346b2e

…libTiCore

Fix sorted imports

31b1357

hansemannn added ios improvement BREAKING CHANGES ⚠️ labels Feb 4, 2018

hansemannn added this to the 8.0.0 milestone Feb 4, 2018

hansemannn requested review from sgtcoolguy and cb1kenobi February 4, 2018 10:19

build added ios needs tests 🚨 labels Feb 4, 2018

Fix typo

ee8bd34

hansemannn added no tests and removed needs tests 🚨 labels Feb 4, 2018

build added the ios label Feb 4, 2018

hansemannn and others added 2 commits February 4, 2018 16:58

Another typo

b42f740

Merge branch 'master' into remove-ticore

f713a31

sgtcoolguy reviewed Feb 6, 2018

View reviewed changes

hansemannn and others added 6 commits February 6, 2018 17:29

Remove TiProfiler and TiDebugger, cleanup build scripts

b8cc542

Merge branch 'master' into remove-ticore

3ea5e1e

Merge branch 'master' into remove-ticore

dd968e6

Merge branch 'master' into remove-ticore

c864ca4

Fix so Ti.UI.TextArea.editable defaults to true (matching our docs).

340f65b

Don't try to copy headers folder that no longer exists. Clean up unus…

6ca4155

…ed code in iOS build script.

build added android ios labels Feb 8, 2018

Merge branch 'master' into remove-ticore

d1256d9

hansemannn closed this May 14, 2018

hansemannn removed this from the 8.0.0 milestone Jun 19, 2018

hansemannn reopened this Jul 20, 2018

hansemannn and others added 3 commits July 20, 2018 21:25

Merge branch 'master' into remove-ticore

d5060c8

Resolve remaining merge conflicts

01593be

Linting

a99030b

hansemannn added this to the 8.0.0 milestone Jul 20, 2018

hansemannn removed the android label Jul 20, 2018

hansemannn and others added 4 commits July 20, 2018 21:58

Update PlatformModule.m

5502a7d

Update TiDOMDocumentProxy.m

378f387

Merge branch 'master' into remove-ticore

292312d

Fix formatting

53562aa

sgtcoolguy suggested changes Jul 25, 2018

View reviewed changes

hansemannn closed this Jul 25, 2018

hansemannn reopened this Jul 25, 2018

hansemannn changed the base branch from master to next July 25, 2018 15:34

tidev deleted a comment from hansemannn Jul 25, 2018

hansemannn and others added 7 commits July 25, 2018 22:28

Restore package-lock.json, restore iOS CLI option

823f6e4

Use Hyperloop 3.1.0

5ed1827

[TIMOB-26204] Android: Fix transformAfterLayout NPE (tidev#10202)

dcc8cd6

[TIMOB-26211] Round percentage up instead of down (parity) (tidev#10194)

dbc3dda

Migrate tests to suite

0c18778

Bump to 8.0.0, publish 'next' branch.

f6d0a07

build added the android label Jul 26, 2018

sgtcoolguy approved these changes Jul 26, 2018

View reviewed changes

sgtcoolguy merged commit 0fcb62d into tidev:next Jul 26, 2018

hansemannn mentioned this pull request Jul 29, 2018

[TIMOB-25927] Add Swift support for native modules, move SDK-core into "TitaniumKit.framework" #10218

Merged

30 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TIMOB-25748] iOS: Remove TiCore in favor of built-in JavaScriptCore #9798

[TIMOB-25748] iOS: Remove TiCore in favor of built-in JavaScriptCore #9798

hansemannn commented Feb 4, 2018 •

edited

build commented Feb 4, 2018 •

edited

sgtcoolguy Feb 6, 2018

hansemannn Feb 6, 2018

sgtcoolguy Feb 6, 2018

sgtcoolguy Feb 6, 2018

hansemannn Feb 6, 2018

hansemannn commented Jul 20, 2018

build commented Jul 20, 2018 •

edited

sgtcoolguy Jul 25, 2018

hansemannn Jul 25, 2018

sgtcoolguy Jul 25, 2018

sgtcoolguy Jul 25, 2018

sgtcoolguy Jul 25, 2018

hansemannn Jul 25, 2018

hansemannn Jul 26, 2018 •

edited

[TIMOB-25748] iOS: Remove TiCore in favor of built-in JavaScriptCore #9798

[TIMOB-25748] iOS: Remove TiCore in favor of built-in JavaScriptCore #9798

Conversation

hansemannn commented Feb 4, 2018 • edited

build commented Feb 4, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hansemannn commented Jul 20, 2018

build commented Jul 20, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hansemannn Jul 26, 2018 • edited

Choose a reason for hiding this comment

hansemannn commented Feb 4, 2018 •

edited

build commented Feb 4, 2018 •

edited

build commented Jul 20, 2018 •

edited

hansemannn Jul 26, 2018 •

edited