Skip to content
This repository has been archived by the owner on Sep 30, 2020. It is now read-only.

require CallExpressions inside MemberExpressions trick group-imports rule #94

Closed
wincent opened this issue Sep 23, 2019 · 5 comments · Fixed by #97
Closed

require CallExpressions inside MemberExpressions trick group-imports rule #94

wincent opened this issue Sep 23, 2019 · 5 comments · Fixed by #97
Assignees
Labels

Comments

@wincent
Copy link
Contributor

wincent commented Sep 23, 2019

Just noticed this while looking at https://github.com/liferay/liferay-js-themes-toolkit/pull/393/files#diff-607f89928a61f2489713663b891f76aa

diff --git a/packages/liferay-theme-tasks/plugin/test/tasks/version.js b/packages/liferay-theme-tasks/plugin/test/tasks/version.js
index 1ebd412..1634a94 100644
--- a/packages/liferay-theme-tasks/plugin/test/tasks/version.js
+++ b/packages/liferay-theme-tasks/plugin/test/tasks/version.js
@@ -10,6 +10,7 @@ var chai = require('chai');
 var del = require('del');
 var fs = require('fs-extra');
 var Gulp = require('gulp').Gulp;
+
 var os = require('os');
 var path = require('path');

@@ -30,11 +31,11 @@ var initCwd = process.cwd();
 var registerTasks;
 var runSequence;

Looks like the Gulp require is tricking the group-imports rule into starting a new group.

@wincent wincent added the bug label Sep 23, 2019
@wincent wincent self-assigned this Sep 23, 2019
@wincent
Copy link
Contributor Author

wincent commented Sep 23, 2019

Similar problem here: https://github.com/liferay/liferay-js-themes-toolkit/pull/393/files#diff-5688ba5971f6c2d3bdf9cf4265442a01

diff --git a/packages/liferay-theme-tasks/tasks/build.js b/packages/liferay-theme-tasks/tasks/build.js
index 5e06621..8fa8470 100644
--- a/packages/liferay-theme-tasks/tasks/build.js
+++ b/packages/liferay-theme-tasks/tasks/build.js
@@ -8,12 +8,13 @@

 const del = require('del');
 const fs = require('fs-extra');
-const _ = require('lodash');
-const path = require('path');
-const plugins = require('gulp-load-plugins')();
 const replace = require('gulp-replace-task');
-const through = require('through2');
+const _ = require('lodash');
+const plugins = require('gulp-load-plugins')();
+
+const path = require('path');
 const PluginError = require('plugin-error');
+const through = require('through2');

 const getBaseThemeDependencies = require('../lib/getBaseThemeDependencies');
 const lfrThemeConfig = require('../lib/liferay_theme_config');

This time the CallExpression is inside another CallExpression.

@wincent
Copy link
Contributor Author

wincent commented Sep 23, 2019

I think the fix here will be two (or maybe three parts):

  1. Teach group-imports to not add those blank lines.
  2. (Definitely) Add a new lint that tells people to prefer destructuring over x = require('foo').x.
  3. (Maybe) Add a new lint that tells people not to immediately call require results (eg. x = require('x')()).

wincent added a commit that referenced this issue Sep 23, 2019
This is a very dumb fix for the two special cases reported in #94. I'd
like to find something a little more general and less hard-coded though,
but it is late and nothing elegant is occurring to me at this time.

Close: #94
@wincent
Copy link
Contributor Author

wincent commented Sep 23, 2019

Crude fix for "1." is up at #97

Will look at "2." (and maybe "3.") later on.

wincent added a commit that referenced this issue Sep 24, 2019
This is a very dumb fix for the two special cases reported in #94. I'd
like to find something a little more general and less hard-coded though,
but it is late and nothing elegant is occurring to me at this time.

Close: #94
wincent added a commit that referenced this issue Sep 24, 2019
wincent added a commit that referenced this issue Sep 24, 2019
wincent added a commit that referenced this issue Sep 24, 2019
@wincent
Copy link
Contributor Author

wincent commented Sep 24, 2019

New "destructure-requires" rule to address "2." is up at: #98

wincent added a commit that referenced this issue Sep 24, 2019
feat: add 'destructure-requires' rule (#94)
wincent added a commit that referenced this issue Sep 24, 2019
@wincent
Copy link
Contributor Author

wincent commented Sep 24, 2019

New "no-require-and-call" rule to address "3." is up at: #99

wincent added a commit that referenced this issue Sep 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant