generate default eslint config from vue cli #601

atomfrede · 2020-05-02T22:23:21Z

With this PR the ap generates a default eslint config for vue + typescript as the vue-cli would create. As of our policy 1, I think thats fine. The huge work ahead to adhere to or define exceptions from the rules.

TODOs:

Cleanup: Remove tslint config

We have with a default app

40 errors and 158 warnings found.

Which seems okay to me. Most warnings (as it seems) are from tests with not used variables.

❓ Can we delete/cleanup old files in a blueprint? So can we overwrite the cleanup.js in a blueprint?

Closes #597
Updates #600

Please make sure the below checklist is followed for Pull Requests.
All continuous integration tests are green
Tests are added where necessary
Documentation is added/updated where necessary
Coding Rules & Commit Guidelines as per our CONTRIBUTING.md document are followed

pascalgrimaud · 2020-05-05T09:19:37Z

Can we delete/cleanup old files in a blueprint? So can we overwrite the cleanup.js in a blueprint?

Do you think it's possible @mshima ?

atomfrede · 2020-06-09T04:53:17Z

generators/client/templates/vue/.eslintrc.js.ejs

+    rules: {
+      'no-console': process.env.NODE_ENV === 'production' ? 'warn' : 'off',
+      'no-debugger': process.env.NODE_ENV === 'production' ? 'warn' : 'off',
+      '@typescript-eslint/no-explicit-any': 'off',


These are just temporary until we have cleaned up the templates

atomfrede · 2020-06-09T04:59:05Z

generators/client/templates/vue/src/main/webapp/app/shared/data/data-utils.service.ts.ejs

        entity[field] = null;
      }
-      if (entity.hasOwnProperty(fieldContentType)) {
+      if (Object.prototype.hasOwnProperty.call(entity, fieldContentType)) {


These are errors from es lint, which are now fixed/

atomfrede · 2020-06-09T04:59:27Z

generators/client/templates/vue/src/main/webapp/app/shared/jhi-item-count.vue.ejs

@@ -3,9 +3,11 @@
        <span v-if="i18nEnabled"
              v-text="$t('global.item-count', {first, second, total })">Showing {{first}} - {{second}} of {{total}} items.</span>
    <span v-if="!i18nEnabled">
+      <!-- eslint-disable vue/no-parsing-error -->          


This rule is disabled as I am pretty sure it is a false positive here.

atomfrede · 2020-06-09T05:01:18Z

@pascalgrimaud I have ignored all warnings for now (a lot of unused vars, missing type declarations etc). If you like you can give this a try. The config is aligned with the default one from vue-cli, so Policy 1 is totally in place :)

pascalgrimaud · 2020-06-09T06:30:21Z

Good job, I'll have a look @atomfrede
For your information, the builds failed because of:

Error: Copying template vue/package.json.ejs failed. [ReferenceError: /home/runner/work/jhipster-vuejs/jhipster-vuejs/generators/client/templates/vue/package.json.ejs:163
    161|     "lint": "vue-cli-service lint",
    162|     "lint:fix": "<%= clientPackageManager %> run lint <%= optionsForwarder %>--fix",
 >> 163|     "cleanup": "rimraf <%= DIST_DIR %> <%= AOT_DIR %>",
    164|     "clean-www": "rimraf <%= DIST_DIR %>app/{src,<%= BUILD_DIR %>}",
    165|     "start": "<%= clientPackageManager %> run webpack:dev",
    166|     "start-tls": "<%= clientPackageManager %> run webpack:dev <%= optionsForwarder %>--env.tls",

AOT_DIR is not defined]
    at module.exports.error (/home/runner/generator-jhipster/generators/generator-base.js:1590:15)
    at /home/runner/generator-jhipster/generators/utils.js:256:23
    at tryHandleCache (/home/runner/generator-jhipster/node_modules/ejs/lib/ejs.js:275:14)
    at Object.exports.renderFile (/home/runner/generator-jhipster/node_modules/ejs/lib/ejs.js:478:10)
    at Object.renderContent (/home/runner/generator-jhipster/generators/utils.js:252:9)

Can you fix it ?

atomfrede · 2020-06-09T06:51:42Z

Good job, I'll have a look @atomfrede
For your information, the builds failed because of:

Error: Copying template vue/package.json.ejs failed. [ReferenceError: /home/runner/work/jhipster-vuejs/jhipster-vuejs/generators/client/templates/vue/package.json.ejs:163
    161|     "lint": "vue-cli-service lint",
    162|     "lint:fix": "<%= clientPackageManager %> run lint <%= optionsForwarder %>--fix",
 >> 163|     "cleanup": "rimraf <%= DIST_DIR %> <%= AOT_DIR %>",
    164|     "clean-www": "rimraf <%= DIST_DIR %>app/{src,<%= BUILD_DIR %>}",
    165|     "start": "<%= clientPackageManager %> run webpack:dev",
    166|     "start-tls": "<%= clientPackageManager %> run webpack:dev <%= optionsForwarder %>--env.tls",

AOT_DIR is not defined]
    at module.exports.error (/home/runner/generator-jhipster/generators/generator-base.js:1590:15)
    at /home/runner/generator-jhipster/generators/utils.js:256:23
    at tryHandleCache (/home/runner/generator-jhipster/node_modules/ejs/lib/ejs.js:275:14)
    at Object.exports.renderFile (/home/runner/generator-jhipster/node_modules/ejs/lib/ejs.js:478:10)
    at Object.renderContent (/home/runner/generator-jhipster/generators/utils.js:252:9)

Can you fix it ?

I have seen it, but thats strange as I did not change something there. But I will have a look.

kaidohallik · 2020-06-09T06:54:49Z

I made #628 for fix. This error is caused by jhipster/generator-jhipster#11907

atomfrede · 2020-06-09T07:06:29Z

Thanks @kaidohallik

mshima · 2020-06-09T14:20:23Z

Do you think it's possible @mshima ?

Saw just now.

Can we delete/cleanup old files in a blueprint? So can we overwrite the cleanup.js in a blueprint?

Is there specific behavior to override cleanup.js?

I suggest to add a task to writing priority:

cleanup() {
  this.deleteDestination('tslint.json');
}

The behavior is a bit different than the generator-base-private's removeFile:

deleteDestination (yeoman-generator) - queues the file to be removed at the conflicts priority.
removeFile (generator-base-private) - removes the file from filesystem with shelljs.

isJhipsterVersionLessThan can be used in conjunction of this.jhipsterOldVersion = this.config.get('jhipsterVersion'); but depends on what stage the following was called.

I would suggest each blueprint to write it's own generatedWith version (at the constructor):

// Assume 6.8.0 if value is not set.
this.oldBlueprintVersion = this.blueprintConfig.get('generatedWith') || '6.8.0';
this.blueprintConfig.set('generatedWith', packageJson.version);

if (semver.lt(this.oldBlueprintVersion, desiredVersion)) {
  ... deleteFile;
}

atomfrede · 2020-06-09T17:54:17Z

Thanks @mshima for the detailed explanation. I think we can go with deleting the not needed tslint config.

@pascalgrimaud rebased against master which removes aot_dir. And updated es lint config of the blueprint code itself.

pascalgrimaud · 2020-06-13T09:03:34Z

Just tried it, and here 2 comments:

npm run lint -> there is an auto fix here, instead of displaying lint error, the behavior has changed here
all vue files uses 2 indents instead of 4, so our generated files will conflict a lot when we'll upgrade our application

atomfrede · 2020-06-13T09:22:30Z

Both should be configurable. Will have a look.

atomfrede · 2020-06-13T11:23:52Z

@pascalgrimaud both should be fixed now.

pascalgrimaud · 2020-06-19T07:54:13Z

@atomfrede : is this PR still in draft ?

atomfrede · 2020-06-19T08:27:21Z

The old tslint config file is not yet removed. Otherwise it's good to be merged. The now ignored rules can be fixed afterwards imho

atomfrede · 2020-06-22T06:52:02Z

Regarding the cleanup we could

Do it here (e.g. look at the current config/package.json to determine the used blueprint version). And create a cleanup procedure like we have for the main generator
Keep the tslint.json config file for now (it is not generated anymore for new apps)
Just try to delete tslint.json

pascalgrimaud · 2020-06-29T08:40:24Z

I'm reviewing this. Here some comments:

After the initial generation, when running npm run lint, I have:

1789 warnings found.
1789 warnings potentially fixable with the `--fix` option.

So, I launch: npm run lint:fix, and I still have:

5 errors and 1416 warnings found.
1413 warnings potentially fixable with the `--fix` option.

I think it's not nice to have so much warning.
So I completed with prettier:

diff --git a/.prettierrc b/.prettierrc
index f9b9911..8e8480b 100644
--- a/.prettierrc
+++ b/.prettierrc
@@ -16,3 +16,6 @@ overrides:
   - files: "*.java"
     options:
       tabWidth: 4
+  - files: "*.vue"
+    options:
+      tabWidth: 4
diff --git a/package.json b/package.json
index cdc807e..d3fdfab 100644
--- a/package.json
+++ b/package.json
@@ -102,7 +102,7 @@
     ]
   },
   "scripts": {
-    "prettier:format": "prettier --write \"{,src/**/,webpack/}*.{md,json,js,ts,tsx,css,scss}\"",
+    "prettier:format": "prettier --write \"{,src/**/,webpack/}*.{md,json,js,ts,tsx,css,scss,vue}\"",
     "lint": "vue-cli-service lint --no-fix",
     "lint:fix": "vue-cli-service lint",
     "cleanup": "rimraf target/classes/static/",

Then, I needed to fix some prettier error, and finally, the result is better:

16 warnings found.
16 warnings potentially fixable with the `--fix` option.

With only 16 warnings, I'm sure we can fix all of them to have 0 warning.

@atomfrede :

we should add prettier to help us here
we should fix the prettier error
we should fix the last warning
what do you think ?

atomfrede · 2020-06-29T08:46:01Z

Sounds good. In my sample application I think I was already on 0 lint warnings/errors but I adapted it manually. Should we have an upgrade notice to manually have the old tslint config file deleted and come up with a general approach how to clean up old files?

pascalgrimaud · 2020-06-29T08:48:46Z

We can simply add it the our release note.
No need to add more code in our blueprint to delete this file...

pascalgrimaud · 2020-06-29T10:10:31Z

@atomfrede : do you want me to finish this PR, with my previous comments ?

atomfrede · 2020-06-29T11:17:35Z

@pascalgrimaud If you have some spare time please go ahead. Otherwise I will pick it up on wednesday most likely.

pascalgrimaud · 2020-06-29T18:04:51Z

So, I didn't manage to go further, as there is no similar method in this blueprint: https://github.com/jhipster/generator-jhipster/blob/master/generators/generator-base-private.js#L1601-L1610

pascalgrimaud · 2020-07-26T20:33:53Z

Sorry for this PR @atomfrede but is it possible to report your commits into generator-jhipster ?
I tried to finish your PR before working on the merge, but I was stuck.

atomfrede · 2020-07-26T20:34:59Z

Sure was already thinking if we finish it here or just in the main generator.

pascalgrimaud · 2020-07-26T20:38:58Z

I prefer generator-jhipster directly, as I think I won't wait months before doing v7 release.
Remember we can do v7-beta release :-)

pascalgrimaud · 2020-11-07T15:31:53Z

closed by jhipster/generator-jhipster#12972

atomfrede mentioned this pull request May 3, 2020

Update Vue dependencies to latest versions #600

Closed

pascalgrimaud mentioned this pull request May 9, 2020

Prettier / eslint: vue files #608

Open

atomfrede force-pushed the 597-migrate-from-tslint-to-eslint branch from 09fbedc to 6d526b7 Compare June 9, 2020 04:52

atomfrede commented Jun 9, 2020

View reviewed changes

atomfrede requested a review from pascalgrimaud June 9, 2020 04:59

atomfrede linked an issue Jun 9, 2020 that may be closed by this pull request

Prettier / eslint: vue files #608

Open

atomfrede added 5 commits June 9, 2020 19:50

generate default eslint config from vue cli

cb0729b

update eslint integration

a35d27a

fix es lint errors

d244c2a

fix all errors, ignore warnings

d24d967

udpate 3rd party dependencies

7cce6d0

atomfrede force-pushed the 597-migrate-from-tslint-to-eslint branch from d3f86f6 to 7cce6d0 Compare June 9, 2020 17:51

atomfrede added 8 commits June 9, 2020 20:05

update eslint config

090f668

ignore empty methods during linting for now

be9a0c6

ignore ts comment

b70edce

ignore protractor config from linting

7f37761

correct ignore setting

1535c61

adapt test template for new linting rules

e3116bf

adapt test template for new linting rules

a3b945e

ignore helper scripts e.g. in docker folder

6ce0d0a

atomfrede force-pushed the 597-migrate-from-tslint-to-eslint branch from eba8836 to 6ce0d0a Compare June 9, 2020 20:57

atomfrede added 2 commits June 13, 2020 13:20

do not auto fix on lint

e908291

use 4 spaces for html/vue indent

8b6ba3b

pascalgrimaud marked this pull request as ready for review June 29, 2020 08:15

Merge branch 'master' into 597-migrate-from-tslint-to-eslint

1f50758

atomfrede mentioned this pull request Sep 22, 2020

Vue 3 jhipster/generator-jhipster#12558

Closed

1 task

pascalgrimaud mentioned this pull request Oct 16, 2020

Drop tslint in favor of eslint. jhipster/generator-jhipster#12764

Closed

1 task

atomfrede mentioned this pull request Nov 6, 2020

[vue] use default eslint for vue instead of tslint jhipster/generator-jhipster#12972

Merged

5 tasks

pascalgrimaud closed this Nov 7, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

generate default eslint config from vue cli #601

generate default eslint config from vue cli #601

atomfrede commented May 2, 2020 •

edited

Loading

pascalgrimaud commented May 5, 2020

atomfrede Jun 9, 2020

atomfrede Jun 9, 2020

atomfrede Jun 9, 2020

atomfrede commented Jun 9, 2020

pascalgrimaud commented Jun 9, 2020 •

edited

Loading

atomfrede commented Jun 9, 2020

kaidohallik commented Jun 9, 2020

atomfrede commented Jun 9, 2020

mshima commented Jun 9, 2020 •

edited

Loading

atomfrede commented Jun 9, 2020 •

edited

Loading

pascalgrimaud commented Jun 13, 2020

atomfrede commented Jun 13, 2020

atomfrede commented Jun 13, 2020

pascalgrimaud commented Jun 19, 2020

atomfrede commented Jun 19, 2020

atomfrede commented Jun 22, 2020

pascalgrimaud commented Jun 29, 2020

atomfrede commented Jun 29, 2020

pascalgrimaud commented Jun 29, 2020

pascalgrimaud commented Jun 29, 2020

atomfrede commented Jun 29, 2020

pascalgrimaud commented Jun 29, 2020

pascalgrimaud commented Jul 26, 2020

atomfrede commented Jul 26, 2020

pascalgrimaud commented Jul 26, 2020

pascalgrimaud commented Nov 7, 2020

generate default eslint config from vue cli #601

generate default eslint config from vue cli #601

Conversation

atomfrede commented May 2, 2020 • edited Loading

pascalgrimaud commented May 5, 2020

atomfrede Jun 9, 2020

Choose a reason for hiding this comment

atomfrede Jun 9, 2020

Choose a reason for hiding this comment

atomfrede Jun 9, 2020

Choose a reason for hiding this comment

atomfrede commented Jun 9, 2020

pascalgrimaud commented Jun 9, 2020 • edited Loading

atomfrede commented Jun 9, 2020

kaidohallik commented Jun 9, 2020

atomfrede commented Jun 9, 2020

mshima commented Jun 9, 2020 • edited Loading

atomfrede commented Jun 9, 2020 • edited Loading

pascalgrimaud commented Jun 13, 2020

atomfrede commented Jun 13, 2020

atomfrede commented Jun 13, 2020

pascalgrimaud commented Jun 19, 2020

atomfrede commented Jun 19, 2020

atomfrede commented Jun 22, 2020

pascalgrimaud commented Jun 29, 2020

atomfrede commented Jun 29, 2020

pascalgrimaud commented Jun 29, 2020

pascalgrimaud commented Jun 29, 2020

atomfrede commented Jun 29, 2020

pascalgrimaud commented Jun 29, 2020

pascalgrimaud commented Jul 26, 2020

atomfrede commented Jul 26, 2020

pascalgrimaud commented Jul 26, 2020

pascalgrimaud commented Nov 7, 2020

atomfrede commented May 2, 2020 •

edited

Loading

pascalgrimaud commented Jun 9, 2020 •

edited

Loading

mshima commented Jun 9, 2020 •

edited

Loading

atomfrede commented Jun 9, 2020 •

edited

Loading