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.

Sign up for GitHub

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

Jump to bottom

[TIMOB-24027] Provide migration help for new properties (2_0_X) #92

Merged
merged 4 commits into from
Oct 17, 2016
Merged

[TIMOB-24027] Provide migration help for new properties (2_0_X) #92

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9f7a3b7
[TIMOB-24027] Provide migration help for new properties
janvennemann Oct 14, 2016
dde28c8
[TIMOB-24027] Rephrase parts of the migration message
janvennemann Oct 17, 2016
13b50a7
[TIMOB-24027] Add comment and move filename definition to top
janvennemann Oct 17, 2016
8dc39f8
[TIMOB-24027] Display migration instructions after build completes
janvennemann Oct 17, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
71 changes: 62 additions & 9 deletions iphone/plugin/hyperloop.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ function HyperloopiOSBuilder(logger, config, cli, appc, hyperloopConfig, builder
this.nativeModules = {};
this.buildSettings = {};
this.headers = null;
this.needMigration = {};
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer needsMigration

Copy link
Contributor Author

Choose a reason for hiding this comment

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

needsMigration would infer that this variable contains a single value, but it actually contains all methods that need migration so i left it at needMigration.

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree, makes sense.


// set our CLI logger
hm.util.setLog(builder.logger);
Expand All @@ -80,7 +81,7 @@ function HyperloopiOSBuilder(logger, config, cli, appc, hyperloopConfig, builder
* called for each JS resource to process them
*/
HyperloopiOSBuilder.prototype.copyResource = function (builder, callback) {
this.patchJSFile(builder.args[1], callback);
this.patchJSFile(builder.args[0], builder.args[1], callback);
};

/**
Expand Down Expand Up @@ -108,7 +109,8 @@ HyperloopiOSBuilder.prototype.run = function run(builder, callback) {
'compileResources',
'generateStubs',
'copyHyperloopJSFiles',
'updateXcodeProject'
'updateXcodeProject',
'displayMigrationInstructions'
], function (err) {
this.logger.info('Finished ' + HL + ' assembly in ' + (Math.round((Date.now() - start) / 10) / 100) + ' seconds');
callback(err);
Expand Down Expand Up @@ -350,9 +352,9 @@ HyperloopiOSBuilder.prototype.detectSwiftVersion = function detectSwiftVersion(c
/**
* Re-write generated JS source
*/
HyperloopiOSBuilder.prototype.patchJSFile = function patchJSFile(file, cb) {
HyperloopiOSBuilder.prototype.patchJSFile = function patchJSFile(sourceFilename, destinationFilename, cb) {
// look for any require which matches our hyperloop system frameworks
var contents = fs.readFileSync(file).toString();
var contents = fs.readFileSync(destinationFilename).toString();

// skip empty content
if (!contents.length) {
Expand All @@ -361,14 +363,14 @@ HyperloopiOSBuilder.prototype.patchJSFile = function patchJSFile(file, cb) {

// parse the contents
// TODO: move all the regex require stuff into the parser
this.parserState = hm.generate.parseFromBuffer(contents, file, this.parserState || undefined);
this.parserState = hm.generate.parseFromBuffer(contents, destinationFilename, this.parserState || undefined);

// empty AST
if (!this.parserState) {
return cb();
}

var relPath = path.relative(this.resourcesDir, file);
var relPath = path.relative(this.resourcesDir, destinationFilename);

// get the result source code in case it was transformed and replace all system framework
// require() calls with the Hyperloop layer
Expand Down Expand Up @@ -439,12 +441,21 @@ HyperloopiOSBuilder.prototype.patchJSFile = function patchJSFile(file, cb) {
return "require('/" + ref + "')";
}.bind(this));

var needMigration = this.parserState.state.needMigration;
if (needMigration.length > 0) {
this.needMigration[sourceFilename] = needMigration;

needMigration.forEach(function(token) {
newContents = newContents.replace(token.objectName + '.' + token.methodName + '()', token.objectName + '.' + token.methodName);
});
}

if (contents === newContents) {
this.logger.debug('No change, skipping ' + chalk.cyan(file));
this.logger.debug('No change, skipping ' + chalk.cyan(destinationFilename));
cb();
} else {
this.logger.debug('Writing ' + chalk.cyan(file));
fs.writeFile(file, newContents, cb);
this.logger.debug('Writing ' + chalk.cyan(destinationFilename));
fs.writeFile(destinationFilename, newContents, cb);
}
};

Expand Down Expand Up @@ -993,6 +1004,48 @@ HyperloopiOSBuilder.prototype.updateXcodeProject = function updateXcodeProject()

};

/**
* Displays migration instructions for certain methods that changed with iOS 10
* and Hyperloop 2.0.0
*
* Can be removed in a later version of Hyperloop
*/
HyperloopiOSBuilder.prototype.displayMigrationInstructions = function displayMigrationInstructions() {
var that = this;

if (Object.keys(this.needMigration).length === 0) {
return;
}

that.logger.error('');
that.logger.error('!!! CODE MIGRATION REQUIRED !!!');
that.logger.error('');
that.logger.error('Due to changes introduced in iOS 10 and Hyperloop 2.0.0 some method calls need');
that.logger.error('to be changed to property access. It seems like you used some of the affected');
Copy link
Contributor

Choose a reason for hiding this comment

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

to be changed to property access. => to be changed regarding the access of properties.

that.logger.error('methods.');
that.logger.error('');
that.logger.error('We tried to fix most of these automatically during compile time. However, we did');
that.logger.error('not touch your original source files. Please see the list below to help you');
that.logger.error('migrate your code.');
that.logger.error('');
that.logger.error('NOTE: Some line numbers and file names shown here are from your compiled Alloy');
that.logger.error('source code and may differ from your original source code.');

Object.keys(this.needMigration).forEach(function (pathAndFilename) {
var tokens = that.needMigration[pathAndFilename];
var shortPathAndFilename = pathAndFilename.replace(that.resourcesDir, 'Resources');
Copy link
Contributor

Choose a reason for hiding this comment

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

Arrange the variables together for better code-style and use error('') to signalize the empty line more clear.

that.logger.error('');
that.logger.error(' File: ' + shortPathAndFilename);
tokens.forEach(function (token) {
var memberExpression = token.objectName + '.' + token.methodName;
var callExpression = memberExpression + '()';
that.logger.error(' Line ' + token.line + ': ' + callExpression + ' -> ' + memberExpression);
});
});

that.logger.error('');
};

/**
* Clean up unwanted files.
* @param {Object} data - The hook payload.
Expand Down