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-24610] Implement ability to transpile #20

Closed
wants to merge 3 commits into from

Conversation

garymathews
Copy link
Contributor

@garymathews garymathews commented Jan 27, 2018

EXAMPLE
...
const transpile = this.cli.tiapp.transpile || false;
const result = jsanalyze.analyzeJs(contents, {
	filename: from,
	minify: this.minifyJS,
	transpile: transpile,
	targets: {
		chrome: '62'
	}
});
...

JIRA Ticket

lib/jsanalyze.js Outdated
@@ -160,6 +161,22 @@ exports.analyzeJs = function analyzeJs(contents, opts) {
// convert the object of symbol names to an array of symbol names
results.symbols = Object.keys(symbols);

// transpile
if (opts.transpile) {
Copy link
Contributor

Choose a reason for hiding this comment

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

May want to consider building up the JS object options for babel.transform so that when we transpire and minify we don't do it in two separate calls.

It's entirely possible that it may not matter, though. I'm just guessing it may have a performance impact.

Might want to try a quick test and see...

lib/jsanalyze.js Outdated
var presets = [ env ];

if (opts.targets) {
targets = [
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is supposed to be:

presets = [ [ env, { targets: opts.targets } ] ];

lib/jsanalyze.js Outdated
];
}

contents = results.contents = babel.transform(contents, {
Copy link
Contributor

Choose a reason for hiding this comment

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

I know this isn't specific to your changes, but I think we may want to consider some options to retain line numbers or generate debug source mappings.

If users choose to transpile and/or minify, the Studio debugger won't know how to map the original source to the generated files for setting breakpoint lines/etc.

I think we've always hacked around this in the past by basically turning off minify when debugging, but we're going to try and always transpile now (even though it may not result in any changes), so it'll be a bigger issue.

Unfortunately, I'm not sure if Babel has fixed their source mapping, as it turned out to be broken when I did the ES6 alloy changes: tidev/alloy#860

The ugly/simple fix is to set options.retainLines = true; which jumps through hoops to keep line numbers the same, but obviously may not really "minify" a file fully if it needs to keep a bunch of empty lines around.

Copy link
Contributor

@sgtcoolguy sgtcoolguy left a comment

Choose a reason for hiding this comment

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

lib/jsanalyze.js#169 fix definitely needs to be done. Consider adding options.retainLines= true; for transpile.

@sgtcoolguy
Copy link
Contributor

Merged manually. I forced retainLines = true for minify and/or transpile (not just minify).

@sgtcoolguy
Copy link
Contributor

Manually merged.

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

2 participants