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: use babel-plugin-transform-titanium when transpiling #11400
Conversation
New dependencies added: babel-plugin-transform-titaniumAuthor: cwilliams Description: Replace known values in Titanium apps to allow for further simplification via dead code/evaluation in babel minify Homepage: http://npmjs.com/package/babel-plugin-transform-titanium
|
20b66b3
to
4a77511
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me. Just one minor nit on the timestamp in prebuild.js
differing to Utils.timestamp
.
Did a small FR and am seeing values get inlined as expected 🎉
android/titanium/prebuild.js
Outdated
@@ -81,13 +81,18 @@ async function generateBuildProperties() { | |||
buildGitHash = 'HEAD'; | |||
} | |||
|
|||
let buildTimestamp = process.env.TI_SDK_BUILD_TIMESTAMP; | |||
if (!buildTimestamp) { | |||
buildTimestamp = moment().format('YYYY/MM/DD HH:mm'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The timestamp generated here is different to the one generated by Utils.timestamp
(2020/01/06 12:10
vs 1/6/2020 12:10
). Does that matter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it may! We should be using the same format in the manifest.json in the SDK as we use in the Ti.buildDate property at runtime (and that value should be the same on either platform). I think that Utils.timestamp basically is what will get shoved into the iOS Ti.buildDate in Obj-c, and will be used for the babel pre-filling. And I think is set to the env var we'll use and prefer above here.
So I think this really only matters for local dev builds, but yeah we should be consistent in our formatting.
I think our unit test doesn't actually verify the format of the value so it's possible we've had iOS/Android differing here for a very long (forever?) time.
4a77511
to
1256e65
Compare
1256e65
to
e7db6cb
Compare
e7db6cb
to
9a11c94
Compare
FR Passed. Studio Ver: 5.1.4.201909061933 |
JIRA: https://jira.appcelerator.org/browse/TIMOB-27167
Description:
This is a PR to make use of appcelerator/babel-plugin-transform-titanium
The goal here is both during SDK build and app build to pass along options that the babel plugin can use to inline static values or during evaluation of special "defines" or common platform/os sniffing expressions.
Relates to tidev/node-titanium-sdk#131.
During SDK build
This PR makes use of the inlining during our generation of the bundled ti.main.js. You can look into the SDK's
dist/tmp/common
folders for the bundledti.main.js
to see the effects (though given we run other transpiration and rollup it can be difficult to know what to look for - easiest thing to spot are removedTi.Platform.name
andTi.Platform.osname
sniffing expressions).This can bake in static constant property values for platforms (think
Ti.buildHash
,Ti.Platform.osname
,Ti.Filesystem.separator
), or replace platform sniffing in ourcommon
JS code.During App build
We pass along options through to node-titanium-sdk to specify the target platform, distribution target, deploy type, etc. So at app build time it can replace even more than simple platform sniffing (think
Ti.App.*
properties).