Skip to content
This repository has been archived by the owner on Feb 27, 2023. It is now read-only.

feat: upgrading version, adding prettier and eslint #27

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jrock2004
Copy link
Collaborator

This PR is setting up for the following things that we want to start working on:

  • Update the versions of the libraries
  • Adding Eslint and Prettier rules

@santthosh-mb
Copy link

santthosh-mb commented Dec 8, 2021

Thank you @jrock2004, really appreciate putting this together. We definitely want to move fast and make progress 👍 .

Ran this template on the local, few issues

  1. Warnings around peer dependencies
yarn add v1.22.11
[1/4] 🔍  Resolving packages...
warning node-sass > [email protected]: request has been deprecated, see https://github.com/request/request/issues/3142
[2/4] 🚚  Fetching packages...
[3/4] 🔗  Linking dependencies...
warning " > [email protected]" has unmet peer dependency "@typescript-eslint/eslint-plugin@^4.0.0".
warning " > [email protected]" has unmet peer dependency "@typescript-eslint/parser@^4.0.0".
warning " > [email protected]" has unmet peer dependency "babel-eslint@^10.0.0".
warning " > [email protected]" has incorrect peer dependency "eslint-plugin-flowtype@^5.2.0".
warning " > @testing-library/[email protected]" has unmet peer dependency "@testing-library/dom@>=7.21.4".
warning " > @storybook/[email protected]" has unmet peer dependency "@babel/core@*".
warning " > @storybook/[email protected]" has unmet peer dependency "@storybook/node-logger@*".
warning "@storybook/preset-create-react-app > [email protected]" has unmet peer dependency "webpack@>= 4".
warning " > @storybook/[email protected]" has unmet peer dependency "@babel/core@^7.9.6".
warning " > @storybook/[email protected]" has unmet peer dependency "babel-loader@^8.0.0".
warning "@storybook/react > @babel/[email protected]" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "@storybook/addon-essentials > @storybook/addon-docs > @storybook/core > @storybook/[email protected]" has unmet peer dependency "webpack@*".
warning "@storybook/react > @babel/[email protected]" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "@storybook/react > @babel/preset-flow > @babel/[email protected]" has unmet peer dependency "@babel/core@^7.0.0-0".
warning "@storybook/react > @babel/preset-flow > @babel/plugin-transform-flow-strip-types > @babel/[email protected]" has unmet peer dependency "@babel/core@^7.0.0-0".
  1. Do we still use husky? I don't know if we want to call that as a pre-requisite
$ husky install
/bin/sh: husky: command not found
error Command failed with exit code 127.
info Visit https://yarnpkg.com/en/docs/cli/add for documentation about this command.
`yarnpkg add @testing-library/jest-dom@^5.14.1 @testing-library/react@^12.0.0 @testing-library/user-event@^13.2.1 @types/jest@^27.0.1 @types/node@^16.7.13 jest-canvas-mock@^2.3.1 cra-shared-ui-scripts@^1.5.1 @storybook/preset-create-react-app@^3.2.0 @storybook/addon-a11y@^6.4.8 @storybook/addon-actions@^6.4.8 @storybook/addon-essentials@^6.4.8 @storybook/addon-links@^6.4.8 @storybook/react@^6.4.8 cross-env@^7.0.3 @commitlint/cli@^15.0.0 @commitlint/config-conventional@^15.0.0 commitizen@^4.2.4 cz-conventional-changelog@^3.3.0 standard-version@^9.3.2 @types/react@^17.0.20 @types/react-dom@^17.0.9 node-sass@^7.0.0 typescript@~4.5.2 mkdirp@^1.0.4 move-cli@^2.0.0 jest-trx-results-processor@^2.2.0 eslint@^7.30.0 eslint-config-airbnb@^18.2.1 eslint-config-prettier@^8.3.0 eslint-config-react-app@^6.0.0 eslint-plugin-flowtype@^6.1.1 eslint-plugin-import@^2.25.2 eslint-plugin-jest@^25.2.2 eslint-plugin-jsx-a11y@^6.4.1 eslint-plugin-prettier@^4.0.0 eslint-plugin-react@^7.26.1 eslint-plugin-react-hooks@^4.2.0 eslint-plugin-react-redux@^3.3.2 eslint-plugin-test-selectors@^1.3.2 prettier@^2.4.1` failed
  1. After installation, got this error around eslint
Error: Failed to load plugin '@typescript-eslint' declared in '.eslintrc.js': Cannot find module '@typescript-eslint/eslint-plugin'
Require stack:
- /Users/Santthosh.Selvadurai/Desktop/Projects/test-app/__placeholder__.js
Referenced from: /Users/Santthosh.Selvadurai/Desktop/Projects/test-app/.eslintrc.js
  1. This is an opportunity to fix README.md and rename all the 'Shared UI' call to 'Micro Frontend', good place to end the confusion that is going around

  2. Should we review and fix the public folder for Mindbody relevance (index.html, logos, favicon.ico etc.,)

  3. yarn start would fail because of babel dependency, probably related to 1.

C02F61GGMD6V:test-app Santthosh.Selvadurai$ yarn start
yarn run v1.22.11
$ react-scripts start

There might be a problem with the project dependency tree.
It is likely not a bug in Create React App, but something you need to fix locally.

The react-scripts package provided by Create React App requires a dependency:

  "babel-loader": "8.1.0"
  1. yarn storybook also fails with the following errors
ERR! Addon value should end in /register OR it should be a valid preset https://storybook.js.org/docs/react/addons/writing-presets/
ERR! storybook-css-modules-preset
  1. Missing .npmrc for us to connect to ADO?

  2. Should we review what goes into cra-shared-ui-scripts as part of this exercise?

  3. It would be great to include appshell-developer and help run things locally

  4. Some teams document gitflow as part of the the README.md and I believe it is very useful

@santthosh-mb santthosh-mb self-assigned this Dec 8, 2021
@jrock2004
Copy link
Collaborator Author

@santthosh-mb Yeah I just copied over things and figured once we discuss and finalize what we want we can clean this stuff up.

@cachilders
Copy link

cachilders commented Dec 8, 2021

  1. Do we still use husky? I don't know if we want to call that as a pre-requisite

We do. Can't say if WE DO.

  1. This is an opportunity to fix README.md and rename all the 'Shared UI' call to 'Micro Frontend', good place to end the confusion that is going around

This is not a change to be made in official documentation until the law is updated, which I have an action item for and requires approval from the Arch leadership. Until the law is changed the official taxonomy is Shared UI with MFE reserved for disambiguation. Let's not add to the confusion by further confusing it. I'll make the law update this week, so the timing may be moot ultimately.

[addendum] The readme references shared ui, which is to be renamed shared component. It is the usage prior to adoption of microfrontends. Shared UI is an MFE. Let's keep our find/replace case-sensitive.

"commit-msg": "commitlint -E HUSKY_GIT_PARAMS"
}
"hooks": {
"commit-msg": "commitlint -E HUSKY_GIT_PARAMS"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is an older way of doing husky commit validation which forces us to pin the husky version in package.json to an old version

new way is detailed here
https://typicode.github.io/husky/#/?id=husky_git_params-ie-commitlint-

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great callout, I forgot to remove this

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

never mind i see you added the commit-msg file below

@@ -0,0 +1,9 @@
{
Copy link
Contributor

@connorkatz-mb connorkatz-mb Dec 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my opinionated config

    "printWidth": 120,
    "tabWidth": 4,
    "semi": true,
    "singleQuote": true,
    "trailingComma": "all",
    "bracketSpacing": true,
    "overrides": [
        {
            "files": ["*.yml"],
            "options": {
                "tabWidth": 2
            }
        }
    ]
}

@cachilders
Copy link

Reckon it's time to close this aged PR.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants