Eslint support #1477 #2843

ResuBaka · 2019-05-01T09:10:34Z

Related issues

Short description and why it's useful

This adds eslint for typescript.
This help to have a more uniform formatting for all JavaScript,Typescript and Vue Files.

Which environment this relates to

Check your case. In case of any doubts please read about [Release Cycle]

Test version (https://test.storefrontcloud.io) - this is a new feature or improvement for Vue Storefront. I've created branch from develop branch and want to merge it back to develop
RC version (https://next.storefrontcloud.io) - this is a stabilisation fix for Release Candidate of Vue Storefront. I've created branch from release branch and want to merge it back to release
Stable version (https://demo.storefrontcloud.io) - this is an important fix for current stable version. I've created branch from hotfix or master branch and want to merge it back to hotfix

Upgrade Notes and Changelog

No upgrade steps required (100% backward compatibility and no breaking changes)
I've updated the Upgrade notes and Changelog on how to port existing VS sites with this new feature

IMPORTANT NOTICE - Remember to update CHANGELOG.md with description of your change

Contribution and currently important rules acceptance

I read and followed contribution rules
I read the TypeScript Action Plan and adjusted my PR according to it
I read about Vue Storefront Modules and I am aware that every new feature should be a module

Some more infos

What still needs or should be done is on the eslint rules.
Here i have tried to have the eslint rules so that there is not formatting changes needed. But here can maybe made some improvements.
@pkarw @filrak @patzick

I know that i should add a changelog but that i am going to add when it es ready to merge and the eslint and prettier rules are set on.

About the broken test

These i am going to fix when i know which rules we want to have in the eslint rules. When the current erros are okay from the rules i can fix them now. But here i want to wait until i know which rules we want to have.

The prettierrc has rules that are like the current style. Only one thing changes with that, that the semicolons rule is true which is the default and that is needed in typescript

With this we get rules that get triggert when prettier was not used. Then it adds all typescript rules for prettier and typescript it self.

…rettier-eslint-support

ResuBaka · 2019-05-01T09:34:30Z

Here is the code when semi is set to true

    const wrapper = mountMixinWithStore(MicrocartProduct, storeMock, {
      propsData: { product }
    });

    (<any>wrapper.vm).removeFromCart();

pkarw · 2019-05-01T16:06:53Z

@ResuBaka please take care of the failing CI checks: https://travis-ci.org/DivanteLtd/vue-storefront/builds/526795740?utm_source=github_status&utm_medium=notification

ResuBaka · 2019-05-01T19:28:08Z

I have now fixed most of them.

I needed to update "@typescript-eslint/eslint-plugin": "^1.7.1-alpha.17", "@typescript-eslint/parser": "^1.7.1-alpha.17" both to a alpha stage which has fixed a problem with semicolons.

Then i have updated the eslint vue package. I have done that because of the update of eslint i have seen that the old version in the package.json had es peer the version ^3 or ^4. That is why i have updatet it.

It would be grade to get some feedback on the still open erros on how some of them should be handelt?
Should they be fixed or should the rule be changed around them.

ResuBaka · 2019-05-02T06:39:44Z

For the v-for with an v-if error this can be fixed with:

https://github.com/vuejs/eslint-plugin-vue/blob/master/docs/rules/no-use-v-if-with-v-for.md#allowusingiterationvar-true

"allowUsingIterationVar": true

Change the loop into a template element. This i would need to test but i thing this would work.

<template v-for="(segment, index) in totals"> 
        <div
          :key="index"
          class="row pt15 pb20 pl30 pr55 "
          v-if="segment.code !== 'grand_total'"
        >

This reverts commit 2c79fa2

This reverts commit b1e7588

Now only some linting issues are open where i don't know how to handel them.

…rettier-eslint-support

ResuBaka · 2019-05-07T15:02:29Z

I have removed the prettier comments from the PR as for now it only adds eslint typescript support.

This was a problem with the rule import/first which i have disabled for this line as it needs to be done after the config is written to disk.

ResuBaka · 2019-05-07T15:10:20Z

To summ all up.

Now all typescript files are linted with eslint.
Most new rules that would be errors are for now set to warning to see if these should be changed back to errors or disabled.
Then there are some cases where it can behave not 100% right as the eslint typescript is not as fare as tslint but they work hard on it.
- For now i only found one case. Which is a problem with not-dev rule, that has problems with interfaces and issue for it can be found here.

lukeromanowicz · 2019-05-10T06:34:28Z

This looks really awesome. The typescript rules look pretty the same as javascript we've been using. Thanks a lot for your hard work!

The only thing I'm concerned about is the code logic after changes because of the instability of ESlint's support for TS. The whole VSF has to be tested carefully after merging this one to ensure that nothing got broken :)

patzick

@ResuBaka thanks for this huge PR:)

Please update changelog.md with your description.

Also i've tried to test this in local env with windows, but yarn command is not passing with node-sass problem, i removed node_modules catalog and still with no success. Can you check if after removing node_modules your yarn command passes?

PS C:\git\vue-storefront> yarn
yarn install v1.13.0
[1/5] Validating package.json...
[2/5] Resolving packages...
[3/5] Fetching packages...
info fsevents@1.2.9: The platform "win32" is incompatible with this module.
info "fsevents@1.2.9" is an optional dependency and failed compatibility check. Excluding it from installation.
[4/5] Linking dependencies...
[5/5] Building fresh packages...
[-/4] ⢀ waiting...
[2/4] ⢀ node-sass
[-/4] ⠠ waiting...
error C:\git\vue-storefront\node_modules\node-sass: Command failed.
Exit code: 1
Command: node scripts/build.js
Arguments:
Directory: C:\git\vue-storefront\node_modules\node-sass
Output:
Building: C:\Program Files\nodejs\node.exe C:\git\vue-storefront\node_modules\node-gyp\bin\node-gyp.js rebuild --verbose --libsass_ext= --libsass_cflags= --libsass_ldflags= --libsass_library=
gyp info it worked if it ends with ok
gyp verb cli [ 'C:\\Program Files\\nodejs\\node.exe',
gyp verb cli   'C:\\git\\vue-storefront\\node_modules\\node-gyp\\bin\\node-gyp.js',
gyp verb cli   'rebuild',
gyp verb cli   '--verbose',
gyp verb cli   '--libsass_ext=',
gyp verb cli   '--libsass_cflags=',
gyp verb cli   '--libsass_ldflags=',
gyp verb cli   '--libsass_library=' ]
gyp info using node-gyp@3.8.0
gyp info using node@10.13.0 | win32 | x64
gyp verb command rebuild []
gyp verb command clean []
gyp verb clean removing "build" directory
gyp verb command configure []
gyp verb check python checking for Python executable "python2" in the PATH
gyp verb `which` failed Error: not found: python2
gyp verb `which` failed     at getNotFoundError (C:\git\vue-storefront\node_modules\which\which.js:13:12)
gyp verb `which` failed     at F (C:\git\vue-storefront\node_modules\which\which.js:68:19)
gyp verb `which` failed     at E (C:\git\vue-storefront\node_modules\which\which.js:80:29)
gyp verb `which` failed     at C:\git\vue-storefront\node_modules\which\which.js:89:16
gyp verb `which` failed     at C:\git\vue-storefront\node_modules\isexe\index.js:42:5
gyp verb `which` failed     at C:\git\vue-storefront\node_modules\isexe\windows.js:36:5
gyp verb `which` failed     at FSReqWrap.oncomplete (fs.js:154:21)
gyp verb `which` failed  python2 { Error: not found: python2
gyp verb `which` failed     at getNotFoundError (C:\git\vue-storefront\node_modules\which\which.js:13:12)
gyp verb `which` failed     at F (C:\git\vue-storefront\node_modules\which\which.js:68:19)
gyp verb `which` failed     at E (C:\git\vue-storefront\node_modules\which\which.js:80:29)
gyp verb `which` failed     at C:\git\vue-storefront\node_modules\which\which.js:89:16
gyp verb `which` failed     at C:\git\vue-storefront\node_modules\isexe\index.js:42:5
gyp verb `which` failed     at C:\git\vue-storefront\node_modules\isexe\windows.js:36:5
gyp verb `which` failed     at FSReqWrap.oncomplete (fs.js:154:21)
gyp verb `which` failed   stack:
gyp verb `which` failed    'Error: not found: python2\n    at getNotFoundError (C:\\git\\vue-storefront\\node_modules\\which\\which.js:13:12)\n    at F (C:\\git\\vue-storefront\\node_modules\\which\\which.js:68:19)\n    at E (C:\\git\\vue-storefront\\node_modules\\which\\which.js:80:29)\n    at C:\\git\\vue-storefront\\node_modules\\which\\which.js:89:16\n    at C:\\git\\vue-storefront\\node_modules\\isexe\\index.js:42:5\n    at C:\\git\\vue-storefront\\node_modules\\isexe\\windows.js:36:5\n    at FSReqWrap.oncomplete (fs.js:154:21)',
gyp verb `which` failed   code: 'ENOENT' }
gyp verb check python checking for Python executable "python" in the PATH
gyp verb `which` succeeded python C:\Users\Patryk\AppData\Local\Programs\Python\Python37-32\python.EXE
gyp ERR! configure error
gyp ERR! stack Error: Command failed: C:\Users\Patryk\AppData\Local\Programs\Python\Python37-32\python.EXE -c import sys; print "%s.%s.%s" % sys.version_info[:3];
gyp ERR! stack   File "<string>", line 1
gyp ERR! stack     import sys; print "%s.%s.%s" % sys.version_info[:3];
gyp ERR! stack                                ^
gyp ERR! stack SyntaxError: invalid syntax
gyp ERR! stack
gyp ERR! stack     at ChildProcess.exithandler (child_process.js:289:12)
gyp ERR! stack     at ChildProcess.emit (events.js:182:13)
gyp ERR! stack     at maybeClose (internal/child_process.js:962:16)
gyp ERR! stack     at Process.ChildProcess._handle.onexit (internal/child_process.js:251:5)
gyp ERR! System Windows_NT 10.0.17134
gyp ERR! command "C:\\Program Files\\nodejs\\node.exe" "C:\\git\\vue-storefront\\node_modules\\node-gyp\\bin\\node-gyp.js" "rebuild" "--verbose" "--libsass_ext=" "--libsass_cflags=" "--libsass_ldflags=" "--libsass_library="
gyp ERR! cwd C:\git\vue-storefront\node_modules\node-sass
gyp ERR! node -v v10.13.0
gyp ERR! node-gyp -v v3.8.0

patzick · 2019-05-13T07:14:28Z

@ResuBaka yarn problem is not related to this issue, i'll test rest when get back my env to working state :)

please for now only update changelog.md

ResuBaka · 2019-05-13T08:38:57Z

Added the linting of typescript files to the changelog.

patzick · 2019-05-13T10:17:15Z

@ResuBaka this yarn problem appears only when i switch to this branch, when im back on develop then it still keeps problem with node-sass in cache but after a while yarn command passes. That's why i thought that problem is with my local environment. Could you check why is this happening?

ResuBaka · 2019-05-13T10:21:24Z

I can check it later on my windows 10 machine.

On my Linux machine node 10.15.3 and yarn 1.16.0 i don't have that problem with and clean git clone and checkout.

When you can tell me your version i will try it later on my Windows 10 setup and see if i get the same problem. @patzick

ResuBaka · 2019-05-13T15:27:17Z

@patzick I have just test it on my windows 10 machine.
Node: 11.5.0
Yarn: 1.12.3

With that setup i have no problem running it.
We could test it in an travis-ci Windows setup and see if it is there, too. As it does not happen for me.

Maybe @pkarw or @filrak can test it and check if someone of them has the same problem.

But for me it looks more like you should try to update node or yarn to a newer version and check if that fixes it. Than an issue with my changes itself.

pkarw · 2019-05-17T11:50:18Z

Hi folks, is it ready to be merged in? @patzick

pkarw · 2019-05-17T11:51:48Z

Please make sure that this PR won't revert the: #2899

The rootStore.state.config is currently deprecated

ResuBaka · 2019-05-17T12:06:26Z

I have accepted all remote changes. So it should not have reverted anything from #2899.

ResuBaka · 2019-05-17T12:42:02Z

I have now fixed the new linting issues.

But there where some strange merge errors (broken typescript code) after i had merged developer. So maybe this should be tested a bit more that nothing is broken now.

Then is it normal that when i run the tests that i see this:

@pkarw.

patzick · 2019-05-23T09:58:00Z

i've merged developp, improved some linting error and i'm merging it
thanks @ResuBaka for this great work!

pkarw · 2019-06-01T05:26:55Z

Hi! I belive I found some issues regarding this feature - @lukeromanowicz @ResuBaka @patzick

yarn eslint which is used by Travis - gets me different errors than yarn build and yarn dev. For instance with yarn eslint I see errors like: https://www.dropbox.com/s/sx6lbscba62nwy0/Screenshot%202019-06-01%2007.25.27.png?dl=0. With the yarn build i've got just the warnings (no errors) which causes CI to fail. We do have some misalligment in here.
The same screen (https://www.dropbox.com/s/sx6lbscba62nwy0/Screenshot%202019-06-01%2007.25.27.png?dl=0) -> it's very hard to get the real issues when 99% of the lines are warnings. We should adjust the linting rules to the real/current code situation. I mean - we probably should skip some checks like unused vars which generates most of the warnings and fix the rest

ResuBaka · 2019-06-01T07:13:27Z

For the second problem we can add a script like lint:full and have the current lint default to eslint --quiet. But that is not a good solution and most warnings should be fixed or later changed to errors when most or all warnings are fixed as they are return types which should be used when you use typescript.

That would for now fix the problem so you can see the errors.

For the first one do you have a branch or commit with wich i could test it or see it? @pkarw

ResuBaka added 4 commits May 1, 2019 10:23

Added prettier with prettierrc

8f29059

The prettierrc has rules that are like the current style. Only one thing changes with that, that the semicolons rule is true which is the default and that is needed in typescript

Added eslint typescript support and updated .eslintrc.js

f47f2f9

With this we get rules that get triggert when prettier was not used. Then it adds all typescript rules for prettier and typescript it self.

Changed prettier parameter path to the right folders

c4d9991

Merge branch 'develop' of github.com:DivanteLtd/vue-storefront into p…

7cd9c42

…rettier-eslint-support

ResuBaka changed the title ~~Prettier eslint support~~ Prettier eslint support #1477 May 1, 2019

pkarw added this to the 1.10.0-rc.1 milestone May 1, 2019

ResuBaka added 3 commits May 1, 2019 20:23

Fixed eslint issue with semicolons

b395e09

Formatted source code with prettier

b1e7588

fixed most of the linting issues

2c79fa2

ResuBaka and others added 8 commits May 4, 2019 11:47

Revert "fixed most of the linting issues"

2fc03f7

This reverts commit 2c79fa2

Revert "Formatted source code with prettier"

b15d99b

This reverts commit b1e7588

Removed prettier and changed most errors to warning for now.

420c7d3

Fixed most of the linting issues

70a8c83

Now only some linting issues are open where i don't know how to handel them.

Merge branch 'develop' of github.com:DivanteLtd/vue-storefront into p…

611d985

…rettier-eslint-support

Fixed the remaning linting errors

253db88

Merge branch 'develop' into prettier-eslint-support

56b362e

fixed eslint errors after merge

09b8e4c

ResuBaka changed the title ~~Prettier eslint support #1477~~ Eslint support #1477 May 7, 2019

Fixed build error for missing config.json

c166ba8

This was a problem with the rule import/first which i have disabled for this line as it needs to be done after the config is written to disk.

lukeromanowicz requested review from pkarw, patzick and filrak May 10, 2019 06:29

lukeromanowicz approved these changes May 10, 2019

View reviewed changes

patzick added 2 commits May 10, 2019 11:59

Merge branch 'develop' into prettier-eslint-support

d15d3df

Merge branch 'develop' into prettier-eslint-support

31a15c7

patzick requested changes May 13, 2019

View reviewed changes

Updated changelog

af669f6

Merge branch 'develop' into prettier-eslint-support

018fba8

Fixed liniting issues

33b31ac

patzick added 4 commits May 23, 2019 10:45

merge develop

bfa0bf9

allow use any type

0d06fa9

fix linter errors

3a64d5c

fix config use

3fdc334

patzick approved these changes May 23, 2019

View reviewed changes

patzick merged commit 1aa03f1 into vuestorefront:develop May 23, 2019

ResuBaka deleted the prettier-eslint-support branch May 23, 2019 11:02

pkarw assigned patzick, ResuBaka and lukeromanowicz Jun 1, 2019

pkarw mentioned this pull request Jun 1, 2019

Enforce coding guidelines with ESLint + TSLint + Prettier #1477

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Eslint support #1477 #2843

Eslint support #1477 #2843

ResuBaka commented May 1, 2019 •

edited

Loading

ResuBaka commented May 1, 2019

pkarw commented May 1, 2019

ResuBaka commented May 1, 2019 •

edited

Loading

ResuBaka commented May 2, 2019

ResuBaka commented May 7, 2019

ResuBaka commented May 7, 2019

lukeromanowicz commented May 10, 2019 •

edited

Loading

patzick left a comment

patzick commented May 13, 2019

ResuBaka commented May 13, 2019

patzick commented May 13, 2019

ResuBaka commented May 13, 2019

ResuBaka commented May 13, 2019

pkarw commented May 17, 2019

pkarw commented May 17, 2019

ResuBaka commented May 17, 2019

ResuBaka commented May 17, 2019

patzick commented May 23, 2019

pkarw commented Jun 1, 2019

ResuBaka commented Jun 1, 2019

Eslint support #1477 #2843

Eslint support #1477 #2843

Conversation

ResuBaka commented May 1, 2019 • edited Loading

Related issues

Short description and why it's useful

Which environment this relates to

Upgrade Notes and Changelog

Contribution and currently important rules acceptance

Some more infos

About the broken test

ResuBaka commented May 1, 2019

pkarw commented May 1, 2019

ResuBaka commented May 1, 2019 • edited Loading

ResuBaka commented May 2, 2019

ResuBaka commented May 7, 2019

ResuBaka commented May 7, 2019

lukeromanowicz commented May 10, 2019 • edited Loading

patzick left a comment

Choose a reason for hiding this comment

patzick commented May 13, 2019

ResuBaka commented May 13, 2019

patzick commented May 13, 2019

ResuBaka commented May 13, 2019

ResuBaka commented May 13, 2019

pkarw commented May 17, 2019

pkarw commented May 17, 2019

ResuBaka commented May 17, 2019

ResuBaka commented May 17, 2019

patzick commented May 23, 2019

pkarw commented Jun 1, 2019

ResuBaka commented Jun 1, 2019

ResuBaka commented May 1, 2019 •

edited

Loading

ResuBaka commented May 1, 2019 •

edited

Loading

lukeromanowicz commented May 10, 2019 •

edited

Loading