Dependencies Update plus small fixes #147

fabiovedovelli · 2021-12-07T14:14:04Z

Changes

Full project dependency update with yarn upgrade-interactive --exact --latest
Installation of @nuxt/types and uuid, after which the project started to build again
Adds testEnvironment: 'jsdom' to jest.config.js, required after jest upgrade
Adds a missing stylesheet in the RequestInfo.vue component to properly display formatted JSON output

Testing

I've been manually testing the application for the past hour and so far everything is working as expected.

Reviewers

@MasterXen @kristinfritsch @jiacovino-circle @MasterXen @tgolbs-circle @felipelima-circle @MehediH @dave-circle @neilkumar-circle @fn0rth

- UUID - @nuxt/types

To add missing stylesheet for Vue JSON Pretty component

kristinfritsch · 2021-12-07T15:19:55Z

I get eslint-plugin-vue@8.2.0: The engine "node" is incompatible with this module. Expected version "^12.22.0 || ^14.17.0 || >=16.0.0". Got "12.13.0" when deleting node_modules and running yarn install

fabiovedovelli · 2021-12-07T15:47:24Z

I get eslint-plugin-vue@8.2.0: The engine "node" is incompatible with this module. Expected version "^12.22.0 || ^14.17.0 || >=16.0.0". Got "12.13.0" when deleting node_modules and running yarn install

Do you have a suggestion on how to proceed?

Shall we document this requirement somewhere?

kristinfritsch · 2021-12-07T17:45:11Z

Shall we document this requirement somewhere?

The node version is pinned here: https://github.com/circlefin/payments-sample-app/blob/master/.nvmrc

To bump Node version to the current LTS

fabiovedovelli · 2021-12-07T18:11:29Z

Shall we document this requirement somewhere?

The node version is pinned here: https://github.com/circlefin/payments-sample-app/blob/master/.nvmrc

It's done!

itsikcircle · 2021-12-07T19:22:15Z

Increasing the node version can break it for someone who runs it on old node, I'm all in on upgrading versions, but just want to make sure we aware of the output of it.

fabiovedovelli · 2021-12-08T10:39:08Z

Increasing the node version can break it for someone who runs it on old node, I'm all in on upgrading versions, but just want to make sure we aware of the output of it.

If the version is pinned in the .nvm it becomes part of the project's prerequisites. We can improve it by clearly stating it in the docs and sending communication to everyone using the app (if possible).

@MasterXen We could use your input in the matter. Thanks!

MasterXen · 2021-12-08T13:12:54Z

Agree that we should update to avoid warnings and product a clean build/runtime. But upgrades should not exceed the LTS version

To fix Node version to 16.3.0 instead of 16.3.1. 16.3.0 is the version we have a Docker image for.

fabiovedovelli · 2021-12-08T16:44:14Z

@itsikcircle @kristinfritsch @MasterXen Node version fixed on 16.3.0 which is the same image we're using on account-web and an image we're providing ourselves.

itsikcircle · 2021-12-08T16:49:25Z

Build fail...

MasterXen · 2021-12-09T05:01:01Z

I've retested the build manually in CodeBuild. There's a few new warnings related to the Nuxt upgrade which seem relatively harmless. However, the build looks to fail on eslint now:

$ eslint --ext .ts,.js,.vue .
--
395 |  
396 | Oops! Something went wrong! :(
397 |  
398 | ESLint: 8.4.1
399 |  
400 | Error: Cannot read config file: /opt/circle/payments-sample-app/src/node_modules/eslint-config-prettier/vue.js
401 | Error: "prettier/vue" has been merged into "prettier" in eslint-config-prettier 8.0.0. See: https://github.com/prettier/eslint-config-prettier/blob/main/CHANGELOG.md#version-800-2021-02-21

kristinfritsch · 2021-12-09T12:54:18Z

~~Apps starts for me locally now, but there is an issue with making requests~~ Works for me locally now! Just need to fix eslint 👯

…lli/payments-sample-app into deps-update-07122021

fabiovedovelli · 2021-12-09T15:50:23Z

.eslintrc.js

+  overrides: [
+    {
+      files: ['*.vue'],
+      rules: {
+        indent: 'off',
+      },
+    },
+  ],


This disables default ESLint's indent rule for Vue files, letting vue/script-indent do its job.

fabiovedovelli · 2021-12-09T15:51:06Z

.eslintrc.js

@@ -22,7 +22,17 @@ module.exports = {
    'vue/html-self-closing': 'off',
    'vue/singleline-html-element-content-newline': 'off',
    'vue/max-attributes-per-line': 'off',
+    'vue/multi-word-component-names': 'off',


This rule doesn't make sense in Nuxt apps because of the filename based routing system. We want to have single name components in order to have tidy routes.

fabiovedovelli · 2021-12-09T15:52:51Z

components/CVVInput.vue

@@ -14,6 +14,7 @@

 <script lang="ts">
 import { Component, Prop, Vue } from 'nuxt-property-decorator'
+// eslint-disable-next-line import/named


I couldn't find a configuration based solution for this warning, so I am disabling it in the 3 places where it's happening. I've checked the lib source code, and it's being named exported.

fabiovedovelli · 2021-12-09T15:54:34Z

nuxt.config.js

  server: {
-    host: '0.0.0.0',
+    host: 'localhost',


This should fix the URL suggestion in the Terminal output. Now it is http://localhost:3000

Thanks, i like that better than #148

fabiovedovelli · 2021-12-09T15:55:06Z

pages/flow/charge/index.vue

-interface CreateChargePayload {
-  id: string
-  amount: {
-    amount: number
-    currency: string
-  }
-  verification: string
-  source: {
-    id: string
-    type: string
-  }
-  keyId: string
-  encryptedData: string
-}
-


ESLint caught this unused piece of code

kristinfritsch · 2021-12-09T18:27:32Z

@fabiovedovelli is currently looking into an issue with the eslint config in devtools/node

fabiovedovelli · 2021-12-10T11:41:09Z

@fabiovedovelli is currently looking into an issue with the eslint config in devtools/node

This is now ready for another review/tests! Thank you

fabiovedovelli · 2021-12-10T11:42:21Z

devtools/node/.eslintrc.js

@@ -0,0 +1,19 @@
+module.exports = {
+  root: true,


This is what prevents ESLint configurations to clash.

kristinfritsch · 2021-12-10T13:48:32Z

Thanks for addressing all the issues! Looks great locally!

fabiovedovelli · 2021-12-10T15:39:35Z

@itsikcircle @kristinfritsch Everything is fixed and this PR is ready for approval. Thanks

fabiovedovelli added 3 commits December 7, 2021 14:49

Updates dependencies and install missing ones

93cf862

- UUID - @nuxt/types

Introduces testEnvironment for jest

09542bb

Update RequestInfo.vue

225fbb2

To add missing stylesheet for Vue JSON Pretty component

kristinfritsch changed the title ~~[BRAAV-7475] - Dependencies Update plus small fixes~~ Dependencies Update plus small fixes Dec 7, 2021

Update .nvmrc

670abd5

To bump Node version to the current LTS

itsikcircle previously approved these changes Dec 8, 2021

View reviewed changes

Update .nvmrc

f87d9eb

To fix Node version to 16.3.0 instead of 16.3.1. 16.3.0 is the version we have a Docker image for.

fabiovedovelli dismissed itsikcircle’s stale review via f87d9eb December 8, 2021 16:41

kristinfritsch added 2 commits December 9, 2021 13:46

Fix eslint configuration

771b4f7

Fix nuxt warnings

98edc2b

This comment has been minimized.

Sign in to view

Revert

67f5229

kristinfritsch mentioned this pull request Dec 9, 2021

Fix api url in local dev #148

Closed

fabiovedovelli added 2 commits December 9, 2021 16:47

Fix all ESLint errors and warnings

11ece23

Merge branch 'deps-update-07122021' of https://github.com/fabiovedove…

8993a57

…lli/payments-sample-app into deps-update-07122021

fabiovedovelli commented Dec 9, 2021

View reviewed changes

Upgrade /devtools/node dependencies and tweat ESLint config

249d35c

fabiovedovelli commented Dec 10, 2021

View reviewed changes

fabiovedovelli added 2 commits December 10, 2021 14:38

Delete package-lock.json

1169107

README update

ee8edec

kristinfritsch approved these changes Dec 10, 2021

View reviewed changes

kristinfritsch merged commit 11b7328 into circlefin:master Dec 10, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dependencies Update plus small fixes #147

Dependencies Update plus small fixes #147

fabiovedovelli commented Dec 7, 2021

kristinfritsch commented Dec 7, 2021

fabiovedovelli commented Dec 7, 2021

kristinfritsch commented Dec 7, 2021

fabiovedovelli commented Dec 7, 2021

itsikcircle commented Dec 7, 2021

fabiovedovelli commented Dec 8, 2021

MasterXen commented Dec 8, 2021

fabiovedovelli commented Dec 8, 2021

itsikcircle commented Dec 8, 2021

MasterXen commented Dec 9, 2021

kristinfritsch commented Dec 9, 2021 •

edited

Loading

This comment has been minimized.

fabiovedovelli Dec 9, 2021

fabiovedovelli Dec 9, 2021

fabiovedovelli Dec 9, 2021

fabiovedovelli Dec 9, 2021

kristinfritsch Dec 9, 2021

fabiovedovelli Dec 9, 2021

kristinfritsch commented Dec 9, 2021

fabiovedovelli commented Dec 10, 2021

fabiovedovelli Dec 10, 2021

kristinfritsch commented Dec 10, 2021

fabiovedovelli commented Dec 10, 2021

Dependencies Update plus small fixes #147

Dependencies Update plus small fixes #147

Conversation

fabiovedovelli commented Dec 7, 2021

Changes

Testing

Reviewers

kristinfritsch commented Dec 7, 2021

fabiovedovelli commented Dec 7, 2021

kristinfritsch commented Dec 7, 2021

fabiovedovelli commented Dec 7, 2021

itsikcircle commented Dec 7, 2021

fabiovedovelli commented Dec 8, 2021

MasterXen commented Dec 8, 2021

fabiovedovelli commented Dec 8, 2021

itsikcircle commented Dec 8, 2021

MasterXen commented Dec 9, 2021

kristinfritsch commented Dec 9, 2021 • edited Loading

This comment has been minimized.

fabiovedovelli Dec 9, 2021

Choose a reason for hiding this comment

fabiovedovelli Dec 9, 2021

Choose a reason for hiding this comment

fabiovedovelli Dec 9, 2021

Choose a reason for hiding this comment

fabiovedovelli Dec 9, 2021

Choose a reason for hiding this comment

kristinfritsch Dec 9, 2021

Choose a reason for hiding this comment

fabiovedovelli Dec 9, 2021

Choose a reason for hiding this comment

kristinfritsch commented Dec 9, 2021

fabiovedovelli commented Dec 10, 2021

fabiovedovelli Dec 10, 2021

Choose a reason for hiding this comment

kristinfritsch commented Dec 10, 2021

fabiovedovelli commented Dec 10, 2021

kristinfritsch commented Dec 9, 2021 •

edited

Loading