[TIMOB-23948] Remove wrench in favor of fs-extra #120

hansemannn · 2017-02-03T13:17:34Z

(Work in Progress)

Note: When building (./build.sh), there are still some warnings due to the titanium dependency:

npm WARN deprecated wrench@1.5.9: wrench.js is deprecated! You should check out fs-extra (https://github.com/jprichardson/node-fs-extra) for any operations you were using wrench for. Thanks for all the usage over the years.
npm WARN deprecated node-uuid@1.4.7: use uuid module instead
npm WARN deprecated wrench@1.5.8: wrench.js is deprecated! You should check out fs-extra (https://github.com/jprichardson/node-fs-extra) for any operations you were using wrench for. Thanks for all the usage over the years.
npm WARN deprecated tough-cookie@2.2.2: ReDoS vulnerability parsing Set-Cookie https://nodesecurity.io/advisories/130

But those should rather be replaced in the SDK.

janvennemann

I left some comments about additional stuff that can be replaced now that we use fs-extra

janvennemann · 2017-02-13T11:27:41Z

iphone/plugin/hyperloop.js

@@ -473,7 +472,7 @@ HyperloopiOSBuilder.prototype.generateSourceFiles = function generateSourceFiles
 		return callback();
 	}

-	fs.existsSync(this.hyperloopJSDir) || wrench.mkdirSyncRecursive(this.hyperloopJSDir);
+	fs.existsSync(this.hyperloopJSDir) || fs.mkdirsSync(this.hyperloopJSDir);


Replace this with ensureDirSync like you did in line 654

janvennemann · 2017-02-13T11:28:59Z

metabase/ios/test/generate_test.js

 		}
-		wrench.mkdirSyncRecursive(buildDir);
+		fs.mkdirsSync(buildDir);


You can replace this block with emptyDirSync

janvennemann · 2017-02-13T11:32:56Z

tools/ci.js

@@ -30,7 +29,7 @@ function downloadURL(url, callback) {

 	var tempName = temp.path({ suffix: '.zip' }),
 		tempDir = path.dirname(tempName);
-	fs.existsSync(tempDir) || wrench.mkdirSyncRecursive(tempDir);
+	fs.existsSync(tempDir) || fs.removeSync(tempDir);


I guess you accidentally replaced mkdirSyncRecursive with removeSync? Also ensureDirSync is probably a good use here.

janvennemann · 2017-02-13T11:33:45Z

tools/ci.js

 			}
-			wrench.mkdirSyncRecursive(buildTempDir);
+			fs.mkdirsSync(buildTempDir);


Replace this block with emptyDirSync

janvennemann · 2017-02-13T11:37:40Z

android/plugins/hyperloop/hooks/android/hyperloop.js

 		}

 		// create a temporary hyperloop directory
 		hyperloopBuildDir = path.join(buildDir, 'hyperloop', 'android');
 		if (!afs.exists(hyperloopBuildDir)) {
-			wrench.mkdirSyncRecursive(hyperloopBuildDir);
+			fs.mkdirsSync(hyperloopBuildDir);


Blocks like this can be replaced with ensureDirSync

janvennemann · 2017-02-13T11:38:23Z

android/plugins/hyperloop/hooks/android/hyperloop.js

 		}
 		if (!afs.exists(resourcesDir)) {
 			fs.mkdirSync(resourcesDir);
 		}
 		// Wipe hyperloop resources each time, we will re-generate
 		if (afs.exists(hyperloopResources)) {
-			wrench.rmdirSyncRecursive(hyperloopResources);
+			fs.removeSync(hyperloopResources);


Blocks like this can be replaced with emptyDirSync

emptyDirSync only empties the directory, it doesn't delete it.

Yeah you are right, i didn't check it will only get created if needed. Just ignore this comment.

hansemannn · 2017-02-16T13:27:00Z

@janvennemann All addressed.

janvennemann

Found a few other obsolete if blocks that can be replaced with fs-extra functionality

janvennemann · 2017-02-23T15:41:27Z

android/plugins/hyperloop/hooks/android/hyperloop.js

 			}
 			// Create destination dir
-			wrench.mkdirSyncRecursive(extractedDir);
+			fs.mkdirsSync(extractedDir);


Replace if block above and mkdirSync with emptyDirSync

janvennemann · 2017-02-23T15:42:43Z

android/plugins/hyperloop/hooks/android/metabase/generate.js

@@ -170,7 +169,7 @@ function generateFromJSON(dir, metabaseJSON, classes, callback) {
 		packages = {};

 	if (fs.existsSync(dir)) {
-		wrench.rmdirSyncRecursive(dir);
+		fs.removeSync(dir);
 	}
 	fs.mkdirSync(dir);


Replace this and the if block with emptyDirSync

janvennemann · 2017-02-23T15:43:38Z

iphone/plugin/hyperloop.js

@@ -178,7 +177,7 @@ HyperloopiOSBuilder.prototype.validate = function validate() {
 HyperloopiOSBuilder.prototype.setup = function setup() {
 	// create a temporary hyperloop directory
 	if (!fs.existsSync(this.hyperloopBuildDir)) {
-		wrench.mkdirSyncRecursive(this.hyperloopBuildDir);
+		fs.mkdirsSync(this.hyperloopBuildDir);
 	}


Replace with ensureDirSync

janvennemann · 2017-02-23T15:44:47Z

tools/ci.js

 			}
-			wrench.mkdirSyncRecursive(buildTempDir);
+			fs.emptyDirSync(buildTempDir);


The if block is not necessary when using emptyDirSync

janvennemann · 2017-02-23T15:47:14Z

android/plugins/hyperloop/hooks/android/hyperloop.js

@@ -551,7 +548,7 @@ exports.cliVersion = '>=3.2';

 			filesDir = path.join(hyperloopBuildDir, 'js');
 			if (!afs.exists(filesDir)) {
-				wrench.mkdirSyncRecursive(filesDir);
+				fs.mkdirsSync(filesDir);
 			}


Replace if block with ensureDirSync

hansemannn · 2017-03-02T08:02:25Z

Addressed

[TIMOB-23948] Remove wrench with fs-extra

e7b64a2

janvennemann requested changes Feb 13, 2017

View reviewed changes

[TIMOB-23948] Update fs methods

7202cb3

janvennemann requested changes Feb 23, 2017

View reviewed changes

[TIMOB-23948] Update fs methods

0215569

janvennemann approved these changes Mar 3, 2017

View reviewed changes

janvennemann merged commit e0f6d8c into tidev:master Mar 3, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[TIMOB-23948] Remove wrench in favor of fs-extra #120

[TIMOB-23948] Remove wrench in favor of fs-extra #120

hansemannn commented Feb 3, 2017 •

edited

janvennemann left a comment

janvennemann Feb 13, 2017

janvennemann Feb 13, 2017

janvennemann Feb 13, 2017

janvennemann Feb 13, 2017

janvennemann Feb 13, 2017

janvennemann Feb 13, 2017

hansemannn Feb 13, 2017

janvennemann Feb 14, 2017

hansemannn commented Feb 16, 2017 •

edited

janvennemann left a comment

janvennemann Feb 23, 2017

janvennemann Feb 23, 2017

janvennemann Feb 23, 2017

janvennemann Feb 23, 2017

janvennemann Feb 23, 2017

hansemannn commented Mar 2, 2017

[TIMOB-23948] Remove wrench in favor of fs-extra #120

[TIMOB-23948] Remove wrench in favor of fs-extra #120

Conversation

hansemannn commented Feb 3, 2017 • edited

janvennemann left a 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

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 Feb 16, 2017 • edited

janvennemann left a 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

Choose a reason for hiding this comment

hansemannn commented Mar 2, 2017

hansemannn commented Feb 3, 2017 •

edited

hansemannn commented Feb 16, 2017 •

edited