Skip to content
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

Initial boilerplate migration to Typescript #62

Open
wants to merge 35 commits into
base: master
Choose a base branch
from

Conversation

gomezhyuuga
Copy link

I'll be working with Typescript so I migrated my codebase to react-scripts-ts

Next, I'll be working on the routing.

Adding Meyer's CSS reset to the index
- Don't check too strictly variable names
- Ignore unused locals (due to react global import, it was always
warning that it wasn't used)
Defined two routes:
- /home
- /favs
Default redirect to /home
In order to enable ES5 iteration support used by Redux Saga
Adding boilerplate files for Redux Saga implementation.
API#getTrending()
API#search(query: string)
Interface for a GIF Object, more details about the schema in:
https://developers.giphy.com/docs/#rendition-guide
Example using raw Component State
Implementation of FETCHED_TRENDING and FETCH_TRENDING
Just convert the favs object into an array and reuse GIFList. The Fav button also works out-of-the-box :)
Use a Middleware to store in localStorage each time the state changes.
Implemented Search into a Saga and into the Component
See `reducers` and FavsSearch component
Creation of Component*.style.ts files
`;

const LINKS = ROUTES.map(({path, content}) =>
<li key={path}><NavLink activeClassName="active" to={path}>{content}</NavLink></li>);

Choose a reason for hiding this comment

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

try to avoid classNames

Copy link
Author

Choose a reason for hiding this comment

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

Thanks...
What could be a workaround in this case for the React Router NavLink?

Choose a reason for hiding this comment

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

you can create NavLink with style using styled components. https://www.styled-components.com/docs/basics#styling-any-components

Copy link
Author

Choose a reason for hiding this comment

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

Thanks!

import * as React from 'react';
import * as ReactDOM from 'react-dom';
import App from './App';
import './normalize.css';

Choose a reason for hiding this comment

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

maybe a good injectGlobal use case

Copy link
Author

Choose a reason for hiding this comment

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

Even for vendor content?

<Header />
<section>
<Switch>
{ROUTES.map(({path, component, exact}) =>
Copy link

Choose a reason for hiding this comment

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

For this specific case where we have few routes that are at the same level the benefit may not be that obvious, but remember that the mindset of React-router 4 is making routing dynamic instead of static. When defining your routes on a separate file you are still approaching this in a static way.

Copy link
Author

Choose a reason for hiding this comment

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

Alright, should I move them to this single file?
Although, the dynamic routing could still be used in any other file connecting with router right?
I'm used to use single-file definitions for routes at the same level and use multiple files (dynamic) for nested routes or with a different context.

import axios from 'axios';

const API_URL = "https://api.giphy.com/v1/gifs";
const API_KEY = process.env.REACT_APP_GIPHY_KEY;
Copy link

Choose a reason for hiding this comment

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

👍

h2 { font-size: 3.6rem; line-height: 1.25; letter-spacing: -.1rem; }

/* HELPERS */
.center { text-align: center }

Choose a reason for hiding this comment

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

this could be a styled component

@@ -0,0 +1,21 @@
import { createAction } from 'redux-actions'
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

@@ -0,0 +1,12 @@
import { Store, Action } from "redux";

const autosave = (store: Store) => (next: any) => (action: Action) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

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.

4 participants