TypeScript template #235

keanulee · 2018-09-27T22:41:42Z

PR opened for review comments only - this branch serves as a template on how to use TypeScript.

Fixes #232

frankiefu

minor comments on typescript stuffs

frankiefu · 2018-09-27T23:49:23Z

src/actions/app.ts

@@ -48,27 +60,27 @@ const loadPage = (page) => (dispatch) => {
  dispatch(updatePage(page));
 };

-const updatePage = (page) => {
+const updatePage: ActionCreator<Action> = (page: string) => {


How about using a more specific type:
const updatePage: ActionCreator<AppActionUpdatePage> = (page: string) => {
or
const updatePage = (page: string): AppActionUpdatePage => {

frankiefu · 2018-09-27T23:49:34Z

src/actions/app.ts

-      opened
-    });
-  }
+export const updateDrawerState: ActionCreator<Action> = (opened: boolean) => {


Use AppActionUpdateDrawerState

frankiefu · 2018-09-27T23:49:38Z

src/actions/counter.ts

+export interface CounterActionIncrement extends Action<'INCREMENT'> {};
+export interface CounterActionDecrement extends Action<'DECREMENT'> {};
+export type CounterAction = CounterActionIncrement | CounterActionDecrement;
+
 export const increment = () => {


Add type CounterActionIncrement, e.g. export const increment = (): CounterActionIncrement => {

frankiefu · 2018-09-27T23:51:44Z

src/actions/shop.ts

    dispatch(addToCartUnsafe(productId));
  }
 };

-export const removeFromCart = (productId) => {
+export const removeFromCart: ActionCreator<Action> = (productId) => {


<Action> -> <ShopActionRemoveFromCart>

frankiefu · 2018-09-27T23:52:02Z

src/actions/shop.ts

  return {
    type: REMOVE_FROM_CART,
    productId
  };
 };

-export const addToCartUnsafe = (productId) => {
+export const addToCartUnsafe: ActionCreator<Action> = (productId) => {


ShopActionAddToCart

abdonrd · 2018-10-04T22:31:19Z

It seems that this template does not work with the new Typescript 3.1.

Ignore the .d.ts, .js & .js.map just in src folder

googlebot · 2018-10-04T22:53:31Z

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of all the commit author(s), set the cla label to yes (if enabled on your project), and then merge this pull request when appropriate.

mercmobily · 2018-10-05T22:23:22Z

So... this is meant to be turned into a "supported" template soon, right?
Is it finished as it is? Or is more testing required?

keanulee · 2018-10-05T22:34:07Z

It's mostly finished. Currently waiting on:

Next pwa-helpers release, where underscore is removed from stateChanged() to match LitElement's convention of no underscore for protected methods (No underscore in connect-mixin API pwa-helpers#43)
Next LitElement release, where LitElement is no longer abstract so we don't need to do the "Connected" element subclassing thing

Haven't looked into the TS 3.1 issues - that's probably for another PR. The process of "releasing" this template will probably just be renaming this branch to template-typescript.

abdonrd · 2018-10-05T23:07:58Z

Next pwa-helpers release, where underscore is removed from stateChanged() to match LitElement's convention of no underscore for protected methods (Polymer/pwa-helpers#43)

About the underscore, you are also going to eliminate it from the properties?

keanulee · 2018-10-05T23:09:03Z

@abdonrd no - the convention is no underscore on protected methods

abdonrd · 2018-10-06T15:32:26Z

By the way, any plan to add watch to the npm start script to recompile on changes?

abdonrd · 2018-10-07T12:51:22Z

Also, any plan to setup some TSLint config?

keanulee · 2018-10-08T17:15:36Z

watch would be nice-to-have for npm start; haven't got around to that yet. PR welcome.

abdonrd · 2018-10-08T17:48:47Z

watch would be nice-to-have for npm start; haven't got around to that yet. PR welcome.

Done at #244!

keanulee · 2018-10-08T19:17:07Z

TS 3.1 issue - blocked on release of Polymer/polymer#5370 (with updated legacy-element-mixin.d.ts)

Update: I think we need this as well - Polymer/polymer#5371

felpsio · 2018-10-10T19:15:14Z

Sugestion: organize better the js files. The number of files of the directory is 4x bigges because of d.ts, js, js.map. Maybe it would be good to create a new directory just to keep the generated files. I think it also would be a good idea to put a watch flag in tsc so people don't need to run "npm start" often to see the changes

mercmobily · 2018-10-10T20:53:18Z

@felipecesar42 I agree with that. That's what lit-html does by setting outDir in tsconfig.json

frankiefu

Looks good!

keanulee · 2018-10-10T21:44:26Z

Ack the number of files comment. Could consider moving TS source to a ts_src/ directory or similar, with the idea that src/ still contains JS so that polymer serve/polymer build continues to work without further changes. Will leave that for another PR.

CLA ok

abdonrd · 2018-10-11T14:04:17Z

src/actions/app.ts

 export const UPDATE_PAGE = 'UPDATE_PAGE';
 export const UPDATE_OFFLINE = 'UPDATE_OFFLINE';
 export const UPDATE_DRAWER_STATE = 'UPDATE_DRAWER_STATE';
 export const OPEN_SNACKBAR = 'OPEN_SNACKBAR';
 export const CLOSE_SNACKBAR = 'CLOSE_SNACKBAR';

-export const navigate = (path) => (dispatch) => {
+export interface AppActionUpdatePage extends Action<'UPDATE_PAGE'> {page: string};


@keanulee why export these interfaces?

Oh, I guess store.js only uses AppAction, not these interfaces. No good reason, but maybe someone might want to create the action before dispatching it?

const action = actionCreator(param); store.dispatch(action);

keanulee added 10 commits September 26, 2018 16:55

WIP

8ee9a2d

Convert some elements to typescript

c4a5c59

TS-supported way of connecting elements

5244b9a

TS-ify shop actions/reducers

e60a74c

Enable TS strict mode

db4d2fa

Remove lazyReducerEnhancer because store types with compose is hard

199ae5d

Use non-null assertions; hide snackbar unless status changes

320d233

Custom typing for compose to enable lazyReducerEnhancer with redux-thunk

c8e7de8

Update to prerelease pwa-helpers

2bc6ed9

Simplify updateDrawerState action

8848a4d

keanulee requested a review from frankiefu as a code owner September 27, 2018 22:41

frankiefu reviewed Sep 28, 2018

View reviewed changes

keanulee added 2 commits September 27, 2018 17:51

Stricter typings for actions/reducers

0d2d34d

Merge branch 'master' into typescript

e535b76

mercmobily mentioned this pull request Oct 3, 2018

Provide a template with TypeScript #232

Closed

Ignore the .d.ts, .js & .js.map just in src folder

40aec21

Merge pull request #238 from abdonrd/patch-1

2d4861f

Ignore the .d.ts, .js & .js.map just in src folder

Merge branch 'master' into typescript

7db2c55

Mark properties/methods private/protected

a15f501

frankiefu approved these changes Oct 10, 2018

View reviewed changes

keanulee changed the base branch from master to template-typescript October 10, 2018 21:40

keanulee changed the title ~~[do not merge] TypeScript template~~ TypeScript template Oct 10, 2018

keanulee merged commit a15f501 into template-typescript Oct 10, 2018

keanulee deleted the typescript branch October 10, 2018 21:47

abdonrd reviewed Oct 11, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TypeScript template #235

TypeScript template #235

keanulee commented Sep 27, 2018 •

edited

Loading

frankiefu left a comment

frankiefu Sep 27, 2018

frankiefu Sep 27, 2018

frankiefu Sep 27, 2018

frankiefu Sep 27, 2018

frankiefu Sep 27, 2018

abdonrd commented Oct 4, 2018

googlebot commented Oct 4, 2018

mercmobily commented Oct 5, 2018

keanulee commented Oct 5, 2018

abdonrd commented Oct 5, 2018 •

edited

Loading

keanulee commented Oct 5, 2018

abdonrd commented Oct 6, 2018

abdonrd commented Oct 7, 2018

keanulee commented Oct 8, 2018

abdonrd commented Oct 8, 2018

keanulee commented Oct 8, 2018 •

edited

Loading

felpsio commented Oct 10, 2018

mercmobily commented Oct 10, 2018

frankiefu left a comment

keanulee commented Oct 10, 2018

abdonrd Oct 11, 2018

keanulee Oct 11, 2018

TypeScript template #235

TypeScript template #235

Conversation

keanulee commented Sep 27, 2018 • edited Loading

frankiefu left a comment

Choose a reason for hiding this comment

frankiefu Sep 27, 2018

Choose a reason for hiding this comment

frankiefu Sep 27, 2018

Choose a reason for hiding this comment

frankiefu Sep 27, 2018

Choose a reason for hiding this comment

frankiefu Sep 27, 2018

Choose a reason for hiding this comment

frankiefu Sep 27, 2018

Choose a reason for hiding this comment

abdonrd commented Oct 4, 2018

googlebot commented Oct 4, 2018

mercmobily commented Oct 5, 2018

keanulee commented Oct 5, 2018

abdonrd commented Oct 5, 2018 • edited Loading

keanulee commented Oct 5, 2018

abdonrd commented Oct 6, 2018

abdonrd commented Oct 7, 2018

keanulee commented Oct 8, 2018

abdonrd commented Oct 8, 2018

keanulee commented Oct 8, 2018 • edited Loading

felpsio commented Oct 10, 2018

mercmobily commented Oct 10, 2018

frankiefu left a comment

Choose a reason for hiding this comment

keanulee commented Oct 10, 2018

abdonrd Oct 11, 2018

Choose a reason for hiding this comment

keanulee Oct 11, 2018

Choose a reason for hiding this comment

keanulee commented Sep 27, 2018 •

edited

Loading

abdonrd commented Oct 5, 2018 •

edited

Loading

keanulee commented Oct 8, 2018 •

edited

Loading