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-24609] ES 6 Support #8972

Merged
merged 12 commits into from May 4, 2017
Merged

[TIMOB-24609] ES 6 Support #8972

merged 12 commits into from May 4, 2017

Conversation

sgtcoolguy
Copy link
Contributor

@sgtcoolguy sgtcoolguy commented Apr 18, 2017

https://jira.appcelerator.org/browse/TIMOB-24609
This ticket is an attempt to support ES6 JS code/syntax on iOS/Android/Windows. Primarily the issue lies with the CLI/tooling being unable to cope with ES6 code (almost entirely due to uglifyjs). This ticket is not about transpiling ES6+ code down to ES5 code, but is meant to support "passing through" ES6+ code in our tooling. Ultimately that means that the OS and JS engine combination determines what is supported, and we aren't arbitrarily enforcing ES5-only due to our own tooling limitations.

@jvandijk
Copy link

Instead of relying on Babel, can Buble be an option for transpiling?

@sgtcoolguy
Copy link
Contributor Author

sgtcoolguy commented Apr 19, 2017

@jvandijk I'm attempting to keep the scope of this PR very narrow - "support" ES6 syntax in user JS code. Specifically, pass it through the tooling without choking. We're not really transpiling with Babel, we're parsing the code to detect Ti.* API usage. And if you have minification enabled, we will use babel/babili to do that. But we're not trying to tackle transpiling ES6 to ES5 at all here (because to some degree we don't really need to, as of Ti 6+ each OS can support ES6 natively, though on iOS you do need to turn on a flag in your tiapp.xml).

I do think it'd be interesting in the future to allow for transpiling in the build to specifically target certain devices/platforms so we can transpile down to the common language set when the target OSes differ in their support levels. But that's a bigger effort, with less immediate gains - and I'd prefer to tackle one thing at a time here.

@sgtcoolguy sgtcoolguy changed the title ES 6 Support [TIMOB-19834] ES 6 Support Apr 19, 2017
@sgtcoolguy
Copy link
Contributor Author

First some caveats for usage/testing:

  • This applies only to classic Titanium apps, as Alloy will still fail on ES6 usage.
  • Support for ES6 varies by OS, OS version and Titanium SDK version. But generally, Titanium 6+ on Android should work, Windows 8.1/10 should work, and iOS 10+ with Titanium 4.1+ should work, with an additional note:
    On iOS, this requires that the tiapp.xml be modified to force use of the embedded JSCore on the iOS device rather than our TiCore port. To do so, edit your tiapp.xml to include the use-jscore-framework tag in the ios section, like below:
<ti:app>
    <ios>
        <use-jscore-framework>true</use-jscore-framework>
    </ios>
</ti:app>

(See http://docs.appcelerator.com/platform/latest/#!/guide/tiapp.xml_and_timodule.xml_Reference-section-src-29004921_tiapp.xmlandtimodule.xmlReference-use-jscore-framework)

iOS 9 with Ti 4.1+ and the XML change above will partially work, see https://kangax.github.io/compat-table/es6/ for the exact compatibility matrix for that combination.

The PR itself includes a handful of ES6 syntax/usages in our test suite, you can manually run what we already test on Jenkins, which is to do:

npm install
cd build
node scons.js cleanbuild android ios
node scons.js test android
node scons.js test ios

@hansemannn
Copy link
Collaborator

hansemannn commented Apr 20, 2017

Question here: So when the developer builds the app with the iOS SDK 10.3, will that version be used to determine which functionality is available or does the device that runs the app?

I'm asking because we support iOS 8+ and this would probably cause issues when the developer adds all the ES6 sugar, but the iOS 8 customers receive crashes. Just wondering if we could use some kind of "compatibility library" to ensure it doesn't happen.

And how do other frameworks solve this problem?

@sgtcoolguy
Copy link
Contributor Author

@hansemannn this is related to what @jvandijk asked above. Basically, this PR does not attempt to transpile code. This PR was to allow user code to "pass through" without our tooling breaking down and failing to parse it, due to uglifyjs limitations. Uglifyjs only can parse ES5 code, so if you write ES6 code the CLI just dies. Babel/babylon support ES6+.

So yes, if you write your app with ES6 code and turn on the JSCore flag and it works fine in iOS 10.3, it will not work fine in iOS 8, because the JS engine on that OS version doesn't support ES6 natively.

Generally speaking, this is exactly what transpilation and Babel are for. We could transpile the user's ES6 code down to ES5 for the actual device. But again, I am intentionally limiting this PR's scope here so we can incrementally improve the situation.

I do think it'd be a useful feature to be able to know our baseline OS/JS engine targets and transpile user code down to what is supported on the minimum targets. But I think that's a larger issue and should be a separate PR/discussion.

@sgtcoolguy
Copy link
Contributor Author

To share what @cb1kenobi asked about in today's standup: This PR looks huge, but is really a one file change, specifically in node_modules/titanium-sdk/lib/jsanalyze.js

There, we parse user JS files to validate them, walk the AST to find Ti.* API usage, and optionally minify the code. My change was to go from using uglifyjs to parse, walk, and minify - to using babylon to parse, babel-traverse/babel-types to walk, and babili to minify.

The rest of the files changed were to add some ES6 unit tests, update the build slightly, and update the node_modules dependencies from the change above.

@sgtcoolguy
Copy link
Contributor Author

Also, this and #8974 are related, and if one gets merged, requires I back port to the other PR.

#8974 is about moving titanium-sdk out to it's own repository. This PR modified a file in that module.

@hansemannn
Copy link
Collaborator

Thanks Chris! Maybe an Epic to support ES6 would help to follow the different stages, since most people will only use it if it can actually be used on all of our supported devices - which is a way bigger PR I guess.

@sgtcoolguy sgtcoolguy changed the title [TIMOB-19834] ES 6 Support [TIMOB-24609] ES 6 Support Apr 20, 2017
@sgtcoolguy
Copy link
Contributor Author

https://jira.appcelerator.org/browse/TIMOB-19834 is an Epic for ES6 support.
I've opened https://jira.appcelerator.org/browse/TIMOB-24609 to represent this PR and the first step of the work involved.

@hansemannn
Copy link
Collaborator

It doesn't seem to work for me. This example code works when started through the source in Xcode, but not in the compiled version. Getting this error:

[ERROR] Failed to parse /Users/hknoechel/Documents/Apps/test_es6/Resources/app.js
[ERROR] Unexpected token: name (Application) [line 1, column 6]
[ERROR]   
[ERROR]     class Application {
[ERROR]     ------^
[ERROR] 

@cb1kenobi
Copy link
Contributor

Looks good! APPROVED

@sgtcoolguy
Copy link
Contributor Author

sgtcoolguy commented Apr 21, 2017

@hansemannn It's hard for me to tell, but if the error is during "build" time, then that's an issue with this PR. If it errors at runtime on the emulator, then it's likely you didn't set the use-jscore-framework flag to true, so the old TiCore JS engine port is choking on the ES6 code.

@hansemannn
Copy link
Collaborator

hansemannn commented Apr 21, 2017

Works for me as well now, sorry for the confusion!

@cb1kenobi
Copy link
Contributor

Looks good! APPROVED

@garymathews
Copy link
Contributor

@sgtcoolguy
Copy link
Contributor Author

@garymathews Ewan pointed that out to me and opened a JIRA for that: https://jira.appcelerator.org/browse/TIMOB-24618

There's also the alloy ticket: https://jira.appcelerator.org/browse/ALOY-1312 covered by PR tidev/alloy#825

@sgtcoolguy sgtcoolguy merged commit 9aa8c9a into tidev:master May 4, 2017
@sgtcoolguy sgtcoolguy deleted the es6 branch May 4, 2017 16:47
sgtcoolguy added a commit to sgtcoolguy/titanium_mobile that referenced this pull request May 4, 2017
* Add unit test that uses es6 syntax to expose our busted tooling

* Move from uglifyjs to babel/babylon to analyze JS files

* Move build's package.json dependencies into top-level devDependencies. Remove uglify-js dependency

* Update node_modules to match new babel usage in place of uglify

* Add more ES6 compatability tests

* Use es6 branch of test suite, which forces iOS to use JSCore framework. Fix relative requires of the es6 test scripts

* Use babili to minify

* Add babeli dependency

* Point to node-titanium-sdk 0.2.0

* Update to node-titanium-sdk 0.2.1
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

5 participants