piotrwitek / typesafe-actions

Typesafe utilities for "action-creators" in Redux / Flux Architecture
https://codesandbox.io/s/github/piotrwitek/typesafe-actions/tree/master/codesandbox
MIT License
2.41k stars 99 forks source link

CRA hangs on typecheck when importing createReducer from another file for combineReducers #173

Closed hsz closed 5 years ago

hsz commented 5 years ago

Description

With the @next release (5.0.0-0), create-react-app hangs on waiting for the typecheck results in a specific case, when an imported map of reducers is passed to combineReducers:

import { combineReducers } from 'redux';
import content from './content';

export default combineReducers({
  content,
});

content.ts

import { createReducer } from 'typesafe-actions';

export default createReducer({});

Above code causes a memory leak and crash at the end. When modified to:

import { combineReducers } from 'redux';
import { createReducer } from "typesafe-actions";

export default combineReducers({
  content: createReducer({}),
});

it still hangs which is a bit interesting - so we still have export default createReducer inside unused/not imported content.ts file. When this file is removed, everything builds correctly.

Steps to Reproduce

Repository contains isolated issue with minimal setup: https://github.com/hsz/typesafe-actions-issue

Run

npm install
npm start

It'll stuck on

Files successfully emitted, waiting for typecheck results...

and emit at the end:

FATAL ERROR: Ineffective mark-compacts near heap limit Allocation failed - JavaScript heap out of memory

Expected behavior

tsc doesn't hang

Suggested solution(s)

Project Dependencies

{
  "compilerOptions": {
    "baseUrl": "./src",
    "target": "es5",
    "lib": [
      "dom",
      "dom.iterable",
      "esnext"
    ],
    "allowJs": true,
    "skipLibCheck": true,
    "esModuleInterop": true,
    "allowSyntheticDefaultImports": true,
    "strict": true,
    "forceConsistentCasingInFileNames": true,
    "module": "esnext",
    "moduleResolution": "node",
    "resolveJsonModule": true,
    "noEmit": true,
    "jsx": "preserve",
    "isolatedModules": true,
    "downlevelIteration": true
  },
  "include": [
    "src"
  ]
}
hsz commented 5 years ago

During some tests, I have removed from node_modules/typesafe-actions/dist/create-reducer.d.ts following line:

declare type HandleActionChainApi<TState, TInputAction extends Action, TRootAction extends Action> = <TActionCreator extends (...args: any[]) => TInputAction, THandledAction extends ReturnType<TActionCreator>, TOutputAction extends Exclude<TInputAction, THandledAction>>(singleOrMultipleCreatorsAndTypes: TActionCreator | TActionCreator[], reducer: (state: TState, action: THandledAction) => TState) => [TOutputAction] extends [never] ? Reducer<TState, TRootAction> & {
    handlers: Record<TRootAction['type'], (state: TState, action: TRootAction) => TState>;
} : Reducer<TState, TRootAction> & {
    handlers: Record<Exclude<TRootAction, TOutputAction>['type'], (state: TState, action: TRootAction) => TState>;
    handleAction: HandleActionChainApi<TState, TOutputAction, TRootAction>;
-   handleType: HandleTypeChainApi<TState, TOutputAction, TRootAction>;
};

Project is building correctly and blazing fast. So it looks like new handleType feature causes this memory leak again.

piotrwitek commented 5 years ago

I checked the issue and there is no memory leak, createReducer cannot be used without at least one handler declared.

So the possible improvement here could be to update type definitions to disallow an incorrect usage.

piotrwitek commented 5 years ago

Here is the solution, will deploy in a few minutes: Kapture 2019-06-28 at 10 47 26

hsz commented 5 years ago

This check will be useful for sure, but when providing at least one action handler, the problem is still valid. I've updated issue repository.

piotrwitek commented 5 years ago

Actually, I noticed another issue, but it is only present in watch mode, normal tsc call works fine. I guess your original issue is related to the watch mode issue.

Not sure what to do with that it kinda seems like a compiler bug.

hsz commented 5 years ago

Yeah, when running CRA app in development mode with npm start, it runs watch mode as well. In general, app is built properly and available in browser, but tsc stucks on

Files successfully emitted, waiting for typecheck results...

and fails at the end because of memory overflow.

hsz commented 5 years ago

The thing is that typesafe-actions@4.4.2 works fine with this setup.

piotrwitek commented 5 years ago

@hsz ok I found and fixed the issue. It was infinite lazy evaluation lookup in recursive type when any type was used.

I'll release fix soon.

hsz commented 5 years ago

any type of what? I'm asking, because in my real-world app I'm trying to cover everything with types and this issue also is present.

piotrwitek commented 5 years ago

@hsz thanks for cooperation helping me to fix that issue. New version was released so you can try now npm i typesafe-actions@next to confirm it is fixed on your side.

piotrwitek commented 5 years ago

When you was using createReducer without passing explicit type arg any is the default action type

piotrwitek commented 5 years ago

Now I realized it should probably be unknown instead of any

hsz commented 5 years ago

So I've upgraded library to 5.0.0-1 and provided RootAction globally as described in docs, but createReducerdoesn't recognize types in handleAction callbacks:

image

Repo upgraded.

piotrwitek commented 5 years ago

You did a small mistake:

interface Types {
    RootAction: RootAction;
  }
hsz commented 5 years ago

You're right - now there is no error, but it still stucks on typecheck.

piotrwitek commented 5 years ago

Yeah, something else will take a look later

piotrwitek commented 5 years ago

@hsz Ok now should be working fine. Please try typesafe-actions@5.0.0-2

I have to disable mix-and-match API so it's either handleType or handleAction but not both in a single reducer

hsz commented 5 years ago

All right, I can confirm that with 5.0.0-2 the issue with CRA and tsc watch is resolved. Thanks!