Skip to content

replaced redux for context, changed some files to ts and added reques… #114

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Mar 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions .eslintrc.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,14 @@
module.exports = {
extends: ['wolox-react'],
extends: ['wolox', 'wolox-react', 'wolox-typescript'],
parser: '@typescript-eslint/parser',
rules: {
'import/no-unresolved': 'off',
'import/no-extraneous-dependencies': 'off'
'import/no-extraneous-dependencies': 'off',
'@typescript-eslint/no-var-requires': 'off'
},
settings: {
react: {
version: 'detect'
}
}
};
8 changes: 0 additions & 8 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -99,15 +99,8 @@ Additional to clean libraries are added
|Name|Description|
|----|-----------|
|[apisauce](https://github.com/infinitered/apisauce)| Talking to APIs doesn't have to be awkward anymore.
|[redux](https://redux.js.org/)| Helps you write applications that behave consistently, run in different environments (client, server, and native), and are easy to test.
|[react-redux](https://react-redux.js.org/)| Is the official React binding for Redux.
|[connected-react-router](https://github.com/supasate/connected-react-router)| A Redux binding for React Router v4.
|[react-router-dom](https://github.com/ReactTraining/react-router/tree/master/packages/react-router-dom)| DOM bindings for React Router.
|[redux-recompose](https://github.com/Wolox/redux-recompose)| A Redux utility belt for reducers and actions. Inspired by acdlite/recompose.
|[redux-form](https://github.com/erikras/redux-form)| A Higher Order Component using react-redux to keep form state in a Redux store.
|[redux-thunk](https://github.com/reduxjs/redux-thunk)| Middlewere for Redux.
|[react-router](https://github.com/ReactTraining/react-router)| Routing for React.
|[redux-beacon](https://github.com/rangle/redux-beacon)| Analytics integration for Redux.
|[seamless-immutable](https://github.com/rtfeldman/seamless-immutable)| Immutable data structures for JavaScript.
|[history](https://www.npmjs.com/package/history)| Manage session history anywhere JavaScript runs.
|[i18next](https://www.i18next.com/)| An internationalization-framework written in and for JavaScript.

Choose a reason for hiding this comment

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

Just a suggestion, if we are going to remove Redux and with that redux-form. Shouldn't we add Formik? Maybe we can have it already installed as a customized library

Copy link
Contributor Author

Choose a reason for hiding this comment

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

maybe, but I won't do it in this PR. As of right now, nothing is actually using redux-form, so I'm not removing any functionality. We should think if we want to force Formik, set it as optional dependency or what do we actually want

Expand All @@ -120,7 +113,6 @@ Also you can select **_optional_** libraries like
|----|-----------|
|[moment](https://momentjs.com/)| Parse, validate, manipulate, and display dates and times in JavaScript.
|[seamless-immutable](https://github.com/rtfeldman/seamless-immutable)| Immutable data structures for JavaScript.
|[reselect](https://github.com/reduxjs/reselect)| Selector library for Redux.
|[babel-module-resolver](https://github.com/tleunen/babel-plugin-module-resolver)| Custom module resolver plugin for Babel.

## Notes
Expand Down
8 changes: 5 additions & 3 deletions generators/app/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,16 +84,18 @@ module.exports.FILES_TO_DESTINATION = [LINTER_IGNORE_PATH, RESCRIPTS_PATH, NPMRC

module.exports.CI_CONFIG_FILE = `${CI_PATH}/config.yml`;

module.exports.BOOTSTRAP_TYPES = [{ name: 'Clean', value: false }, { name: 'Customized', value: true }];
module.exports.BOOTSTRAP_TYPES = [
{ name: 'Clean', value: false },
{ name: 'Customized', value: true }
];

module.exports.DEV_DEPENDENCIES = [
'[email protected]',
'[email protected]',
'[email protected]',
'[email protected]',
'[email protected]',
'[email protected]',
'[email protected].0',
'[email protected].1',
'[email protected]',
'[email protected]',
'[email protected]',
Expand Down
36 changes: 13 additions & 23 deletions generators/app/steps/CustomizableSteps/constants.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
const COMPONENTS_PATH = 'src/app/components';
const SCREENS_PATH = 'src/app/screens';
const CONFIG_PATH = 'src/config';
const REDUX_PATH = 'src/redux';
const UTILS_PATH = 'src/utils';
const SERVICES_PATH = 'src/services';
const CONSTANTS_PATH = 'src/constants';
const DOCS_README_PATH = 'docs';
const DEPENDENCY_SPECIFIC_PATH = 'src/dependency_specific';
const HOOKS_PATH = 'src/app/hooks';
const CONTEXT_PATH = 'src/app/contexts';

module.exports.RESCRIPTS_PATH = {
src: 'rescriptsrc.js',
Expand All @@ -18,40 +19,37 @@ module.exports.NPMRC_PATH = {
destination: '.npmrc'
};

module.exports.REDUX_COMPONENTS = [`${COMPONENTS_PATH}/Field/index.js`];

module.exports.FILES = [
CONFIG_PATH,
CONSTANTS_PATH,
DOCS_README_PATH,
REDUX_PATH,

Choose a reason for hiding this comment

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

Don't we need to change this for a global Context Folder.? Just for global values, like Auth

Copy link
Contributor Author

Choose a reason for hiding this comment

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

global context is in ~app/context and it's already being added in this same object:

module.exports.FILES = [
  ...
  'src/app/reducer.ts',
  'src/app/context.ts',
  ...
]

SCREENS_PATH,
UTILS_PATH,
HOOKS_PATH,
CONTEXT_PATH,
'aws.js',
'tsconfig.json',
'tsconfig.paths.json',
'scripts/deploy.js',
'src/react-app-env.d.ts',
`${COMPONENTS_PATH}/Routes/components/AuthenticatedRoute.tsx`,
Copy link
Contributor

Choose a reason for hiding this comment

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

What happened with the authenticated route?

Copy link
Contributor Author

@damfinkel damfinkel Mar 13, 2020

Choose a reason for hiding this comment

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

I changed it to RouteItem, it's more generic. That component has the potential to receive any condition for being shown or not. For instance, if you have many user types with different access levels

`${COMPONENTS_PATH}/Routes/styles.module.scss`,
'src/app/index.tsx',
`${COMPONENTS_PATH}/Routes`,
`${COMPONENTS_PATH}/Spinner`,
`${COMPONENTS_PATH}/Suspense`,
`${COMPONENTS_PATH}/SearchBar`,
`${COMPONENTS_PATH}/InputLabel`,
`${COMPONENTS_PATH}/TextArea`,
`${COMPONENTS_PATH}/Checkbox`,
`${COMPONENTS_PATH}/RadioGroup`,
`${SERVICES_PATH}/AuthServices.js`,
`${SERVICES_PATH}/AnalyticsService.js`
`${COMPONENTS_PATH}/ProviderWrapper`,
`${SERVICES_PATH}/AuthServices.js`
];

module.exports.TEMPLATE_FILES = [
// TODO: Insert here all template ejs files
'.eslintrc.js',
'.babelrc.js',
'src/index.tsx',
'src/app/index.tsx',
`${COMPONENTS_PATH}/Routes/index.tsx`,
`${SERVICES_PATH}/LocalStorageService.js`
];

Expand All @@ -73,7 +71,6 @@ module.exports.WITHOUT_SEAMLESS_FILES = {
module.exports.OPTIONAL_DEPENDENCIES = {
moment: { dependencies: ['moment@^2.23.0'] },
'seamless-immutable': { dependencies: ['seamless-immutable@^7.1.4'] },
reselect: { dependencies: ['reselect@^4.0.0'] },
'babel-module-resolver': {
dependencies: ['babel-plugin-module-resolver@^3.1.1'],
devDependencies: ['eslint-import-resolver-babel-module@^5.0.0']
Expand All @@ -82,16 +79,8 @@ module.exports.OPTIONAL_DEPENDENCIES = {

module.exports.DEPENDENCIES = [
'apisauce@^1.0.1',
'redux@^4.0.1',
'react-redux@^6.0.0',
'connected-react-router@^6.0.0',
'react-router-dom@^4.3.1',
'redux-recompose@^2.1.0',
'redux-form@^8.0.4',

Choose a reason for hiding this comment

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

Should we add Formik?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

same answer

'redux-thunk@^2.3.0',
'react-router@^4.3.1',
'redux-beacon@^2.0.3',
'@redux-beacon/google-analytics@^1.1.1',
'react-router@^5.1.2',
'react-router-dom@^5.1.2',
'seamless-immutable@^7.1.4',
'history@^4.7.2',
'i18next@^13.0.0',
Expand All @@ -104,9 +93,10 @@ module.exports.DEV_DEPENDENCIES = [
'@types/node@^12.12.14',
'@types/react@^16.9.13',
'@types/react-dom@^16.9.4',
'@types/react-redux@^7.1.5',
'@types/react-router@^5.1.2',
'@types/react-router@^5.1.4',
'@types/react-router-dom@^5.1.3',
'@types/seamless-immutable@^7.1.11',
'@types/webpack-env@^1.14.1'
'@types/webpack-env@^1.14.1',
'eslint-config-wolox-typescript'
];
3 changes: 3 additions & 0 deletions generators/app/steps/CustomizableSteps/copyFiles.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,10 @@ const { FILES, FILES_TO_DELETE, TEMPLATE_FILES, WITHOUT_SEAMLESS_FILES } = requi
module.exports = function copyAllFiles() {
mkdirp(this.destinationPath(`${this.projectName}/src/app/assets/`));

console.log('Deleting default CRA files...');
deleteFiles.bind(this)(FILES_TO_DELETE);

console.log('Adding files...');
Comment on lines +10 to +13
Copy link
Contributor

Choose a reason for hiding this comment

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

Are these on purpose?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I used them to debug but then I thought it improved the output for the user

copyFiles.bind(this)(FILES);
copyTemplateFiles.bind(this)(TEMPLATE_FILES);

Expand Down
12 changes: 10 additions & 2 deletions generators/app/tasks/fileCreators.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,13 @@ const getPackageJsonAttributes = (projectName, projectVersion, repoUrl) => ({
name: projectName,
title: projectName,
version: projectVersion,
jest: {
moduleNameMapper: {
'~screens(.*)': '<rootDir>/src/app/screens/$1',
'~components(.*)': '<rootDir>/src/app/components/$1',
'^~(.*)/(.*)$': '<rootDir>/src/$1/$2'
}
},
repository: {
type: 'git',
url: repoUrl
Expand Down Expand Up @@ -60,10 +67,11 @@ module.exports.createJSConfig = function createJSConfig() {
'~screens/*': ['./src/app/screens/*'],
'~config/*': ['./src/config/*'],
'~constants/*': ['./src/constants/*'],
'~redux/*': ['./src/redux/*'],

Choose a reason for hiding this comment

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

If we add a context folder we should add it here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think the I answered this in the previous comment

'~services/*': ['./src/services/*'],
'~utils/*': ['./src/utils/*'],
'~assets/*': ['./src/assets/*']
'~assets/*': ['./src/assets/*'],
'~hooks/*': ['./src/app/hooks/*'],
'~contexts/*': ['./src/app/contexts/*']
};
}

Expand Down
2 changes: 1 addition & 1 deletion generators/app/tasks/gitConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ module.exports.gitInitiation = function gitInitiation() {
successMessage: 'CRA README.md removed successfully',
failureMessage: 'CRA README.md removing failed, no README.md found'
// catching with an empty function to ignore the error
// eslint-disable-next-line no-empty-function
// eslint-disable-next-line no-empty-function, @typescript-eslint/no-empty-function
}).catch(() => {})
);
};
Expand Down
6 changes: 4 additions & 2 deletions generators/app/templates/.babelrc.ejs
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@ module.exports = {
"~screens": "./src/app/screens",
"~config": "./src/config",
"~constants": "./src/constants",
"~redux": "./src/redux",
"~services": "./src/services",
"~utils": "./src/utils"
"~utils": "./src/utils",
"~app": "./src/app",
"~hooks": "./src/app/hooks",
"~contexts": "./src/app/contexts"
}
}
]<% } %>
Expand Down
42 changes: 8 additions & 34 deletions generators/app/templates/.eslintrc.ejs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
module.exports = {
extends: ['wolox-react'<% if (features && features.customized) { %>, 'plugin:import/typescript'<% } %>],<% if (features && features.customized) { %>
parser: '@typescript-eslint/parser',
plugins: ['@typescript-eslint'],<% } %>
extends: ['wolox-react'<% if (features && features.customized) { %>, 'wolox-typescript'<% } %>],<% if (features && features.customized) { %>
parser: '@typescript-eslint/parser',<% } %>
settings: {
'react': {
'version': 'detect'
Expand All @@ -15,41 +14,16 @@ module.exports = {
'~screens': './src/app/screens',
'~config': './src/config',
'~constants': './src/constants',
'~redux': './src/redux',
'~services': './src/services',
'~utils': './src/utils'
'~utils': './src/utils',
'~app': './src/app',
'~assets': './src/assets',
'~hooks': './src/app/hooks',
'~contexts': './src/app/contexts'
}
}
}<%} %>
}<% if (features && features.customized) { %>,
rules: {
'react/jsx-filename-extension': [1, { 'extensions': ['.js', '.jsx', '.tsx'] }],
'spaced-comment': ['error', 'always', { 'markers': ['/'] }],
'@typescript-eslint/adjacent-overload-signatures': 'error',
'@typescript-eslint/ban-types': 'error',
'camelcase': 'off',
'@typescript-eslint/camelcase': 'error',
'@typescript-eslint/class-name-casing': 'error',
'@typescript-eslint/consistent-type-assertions': 'error',
'@typescript-eslint/member-delimiter-style': 'error',
'no-array-constructor': 'off',
'@typescript-eslint/no-array-constructor': 'error',
'no-empty-function': 'off',
'@typescript-eslint/no-empty-function': 'error',
'@typescript-eslint/no-empty-interface': 'error',
'@typescript-eslint/no-inferrable-types': 'error',
'@typescript-eslint/no-misused-new': 'error',
'@typescript-eslint/no-namespace': 'error',
'@typescript-eslint/no-this-alias': 'error',
'no-unused-vars': 'off',
'@typescript-eslint/no-unused-vars': 'error',
'no-use-before-define': 'off',
'@typescript-eslint/no-use-before-define': 'error',
'@typescript-eslint/no-var-requires': 'error',
'@typescript-eslint/prefer-namespace-keyword': 'error',
'@typescript-eslint/triple-slash-reference': 'error',
'@typescript-eslint/type-annotation-spacing': 'error'
}<% } %><% if (features && features.jest) { %>,
}<% if (features && features.jest) { %>,
env: {
jest: true
}<% } %>
Expand Down
34 changes: 0 additions & 34 deletions generators/app/templates/docs/Field.md

This file was deleted.

33 changes: 1 addition & 32 deletions generators/app/templates/docs/SearchBar.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,35 +24,4 @@
<input name="lastName" id="lastName" type="input" onChange={this.onChange}/>
</label>
</SearchBar>
```

If you decided to use redux-form you must add it to the component and use it in this way.

```jsx
<SearchBar
onSubmit={handleSubmit}
className={styles.userFormContainer}
formClassName={styles.userForm}
>
<div className={styles.search}>
<Field
component={Input}
name="name"
placeholder={i18next.t('SearchUser:namePlaceholder')}
type="text"
/>
<Field
component={Input}
name="brand"
placeholder={i18next.t('SearchUser:brandPlaceholder')}
type="text"
/>
<Field
component={Select}
name="status"
dataOptions={STATES}
placeholder={i18next.t('SearchUser:statusPlaceholder')}
/>
<div>
</SearchBar
```
```
12 changes: 0 additions & 12 deletions generators/app/templates/docs/TextArea.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,15 +20,3 @@
onFocus={this.handleFocus}
/>
```

If you decided to use redux-form you must add it to the component and use it in this way.

```jsx
textArea = wrapField(TextArea);

<Field
name={fieldName}
component={this.textArea}
className={styles.textArea}
/>
```
5 changes: 3 additions & 2 deletions generators/app/templates/jsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@
"~screens/*": ["./src/app/screens/*"],
"~config/*": ["./src/config/*"],
"~constants/*": ["./src/constants/*"],
"~redux/*": ["./src/redux/*"],

Choose a reason for hiding this comment

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

I don't think it is a discussion for now, but should we use ~ or @ in Viajeros we had some problems using ~ as it assumed all of those roots were imported from node modules. In amex we used @ and we could avoid the internal/external package problem using the following eslint configuration.

'import/order': [
      'error', {
        'newlines-between': 'always',
        groups: ['builtin', 'external', ['unknown', 'internal'], 'parent', 'sibling', 'index']
      }
    ]

With this eslint can differenciate between @apollo and @components. I think it could be a good improvment, what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I had issues with jest for some reason when I tried it to use @. Can you add a card in here https://trello.com/b/sZVeiFgw/departamento-de-frontend so we can do this separatedly?

"~services/*": ["./src/services/*"],
"~utils/*": ["./src/utils/*"],
"~assets/*": ["./src/assets/*"]
"~assets/*": ["./src/assets/*"],
"~hooks/*": ["./src/hooks/*"],
"~contexts/*": ["./src/contexts/*"]
}
},
"include": ["src"],
Expand Down
4 changes: 4 additions & 0 deletions generators/app/templates/setupTests.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,7 @@ import { configure } from 'enzyme';
import Adapter from 'enzyme-adapter-react-16';

configure({ adapter: new Adapter() });

jest.mock('i18next', () => ({
t: () => ''
}));
Loading