Skip to content

авторизация #3

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

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

авторизация #3

wants to merge 1 commit into from

Conversation

AlexeySkydan201
Copy link
Collaborator

No description provided.

BrowserRouter,
Route,
Switch,
withRouter,
Copy link
Member

Choose a reason for hiding this comment

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

it seems like you don't use it, so we make remove it

return (
<div>

<Route path = "/home" component={In}/>
Copy link
Member

Choose a reason for hiding this comment

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

add , and exact prop to each Route could be more safer decision

@@ -0,0 +1,6 @@
export const incrementCounter = (payload) => {
Copy link
Member

Choose a reason for hiding this comment

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

why do you need an increment counter?

padding: 0;
font-family: sans-serif;
}
#first {
Copy link
Member

Choose a reason for hiding this comment

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

it's always better to use classNames over id. Using ids is a pitfall for bugs

import {incrementCounter} from '../../actions';


let url3 = 'https://flatearth-api.herokuapp.com/api/v1/auth/login'
Copy link
Member

Choose a reason for hiding this comment

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

url3 is bad name, it could be something like

const authURL = ...

but from architecture point of view it's still no good to give component knowledge about URL, fetching resources, headers, etc. It should be another file, store in folder service with file auth-api

and auth-api should have something like this

// auth-api.js
const URLprefix = 'https://flatearth-api.herokuapp.com/api/v1/auth/'

const authAPI = {
 login(user) {
  const {
   user,
   password
  } = user;
  const loginURL = `${urlPrefix}/login`;

  return fetch(loginURL, {}).then(data => data.json()) // with required headers, meta information, etc  
 }
}

So you just importing such file and you component working with single method.
That knowledge about splitting application for different layers makes huge different from "common" developer and good developer



let mapStateToProps = (state) => {
console.log(`stateghjkgkkg`, state.counter.counter)
Copy link
Member

Choose a reason for hiding this comment

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

please remove logs

token1: state.counter.counter
}
}
export default connect(mapStateToProps)(User);
Copy link
Member

Choose a reason for hiding this comment

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

it's bad practice to export components in such manner without naming

withRouter,
} from "react-router-dom";

//const store = createStore(allReducers);
Copy link
Member

Choose a reason for hiding this comment

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

the same about commented code, we don't need it in production

@@ -0,0 +1,7 @@
import {createStore, applyMiddleware} from 'redux';

Copy link
Member

Choose a reason for hiding this comment

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

please remove blank line



import {reducer} from './reducers/reducer';

Copy link
Member

Choose a reason for hiding this comment

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

please remove blank line

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

Successfully merging this pull request may close these issues.

2 participants