Skip to content
This repository has been archived by the owner on Apr 8, 2020. It is now read-only.

chore(typescript): Upgrade to 2.0 & Types #258

Closed
wants to merge 10 commits into from
Closed

chore(typescript): Upgrade to 2.0 & Types #258

wants to merge 10 commits into from

Conversation

MarkPieszak
Copy link
Contributor

@MarkPieszak MarkPieszak commented Aug 12, 2016

! Work in progress !

Upgraded all Project Templates to TS2.0beta, @types, removed all remnants of tsd, typings folders, etc.
Left Universal specific custom types in types/custom-types.d.ts for the ng2 template.

Note: TypeScript2 is still in beta, Visual Studio / VSCode / etc need to be setup to realize they're using a beta/nightly build of TypeScript to take advantage of 2.0 features.

Tested & Working:

  • Angular2Spa
  • KnockoutSpa
  • ReactSpa
  • WebApplicationBasic

WIP Issues :

ReactReduxSpa

it looks like the project needs to be updated to the latest API, @types here cause many errors, since its based on an older API.

  • [ ](Issues remaining with certain react @types / Store / Dispatch)
    • node_modules/redux-typed/StrongActions.d.ts (16,16): error TS2314: Generic type 'Dispatch<S>' requires 1 type argument(s). (can be fixed with PR Update aspnet-webpack-react to React 15 #256 )
    • @types/react-router-redux/index.d.ts error TS2305: Module '"history"' has no exported member 'LocationDescriptor'. (possible Type definition issue? Opened here)
    • ./ClientApp/boot-client.tsx error TS2322: Type 'ReactRouterReduxHistory' is not assignable to type 'History'. Property 'listenBefore' is missing in type 'ReactRouterReduxHistory'.

ERROR in C:\Users\mark.pieszak\Documents\GitHub\JavaScriptServices\templates\ReactReduxSpa\node_modules\redux-typed\StrongActions.d.ts
(16,16): error TS2314: Generic type 'Dispatch' requires 1 type argument(s).

ERROR in C:\Users\mark.pieszak\Documents\GitHub\JavaScriptServices\templates\ReactReduxSpa\types\custom-types.d.ts
(7,1): error TS2688: Cannot find type definition file for 'react-router/history.d.ts'.

ERROR in ./ClientApp/configureStore.ts
(12,39): error TS2346: Supplied parameters do not match any signature of call target.

ERROR in ./ClientApp/configureStore.ts
(19,75): error TS2686: Identifier 'Redux' must be imported from a module

ERROR in ./ClientApp/configureStore.ts
(19,75): error TS2314: Generic type 'Store' requires 1 type argument(s).

ERROR in ./ClientApp/configureStore.ts
(33,91): error TS2686: Identifier 'Redux' must be imported from a module

@SteveSandersonMS
Copy link
Member

Thanks for this! I'll merge it as soon as TypeScript 2.0 ships. It will be great to get all those .d.ts files out of the project 👍

@MarkPieszak
Copy link
Contributor Author

Glad to help 😄
Sounds like a plan!
Doing some finishes touches on the PR at the moment. Almost finished testing & fixing all the templates.

# Conflicts:
#	templates/Angular2Spa/package.json
#	templates/KnockoutSpa/tsd.json
#	templates/KnockoutSpa/typings/tsd.d.ts
#	templates/ReactReduxSpa/package.json
#	templates/ReactSpa/tsd.json
#	templates/ReactSpa/typings/tsd.d.ts
@MarkPieszak
Copy link
Contributor Author

Just fixed the merged conflicts, @SteveSandersonMS
Can we merge this before merging the Angular2 final + Universal updates?

@stephtr
Copy link
Contributor

stephtr commented Sep 18, 2016

If it helps: "error TS2686: Identifier 'Redux' must be imported from a module" seems to be related with TypeScript #10638. Using typescript@2.1.0-dev.20160918 or the Release-2.0-branch these errors vanish.

"(12,39): error TS2346: Supplied parameters do not match any signature of call target" can be fixed by changing windowIfDefined.devToolsExtension to
windowIfDefined.devToolsExtension as () => (a: any) => any or better: () => GenericStoreEnhancer

@MarkPieszak
Copy link
Contributor Author

MarkPieszak commented Sep 18, 2016

Thank you @stephtr I'll take a look! I wasn't sure if we would want to use typescript@next since it cold e very unstable though?

The supplied parameters one is because compose is using a very old type definition from tsd, when upgrading to types it's using the newest redux, so we need to reconfigure the API :(
Hoping I can spend some time with Steve to look through the ReactRedux changes needed.

Other than that, I was able to get all the other projects running without errors luckily.

@colltoaction
Copy link

Maybe you can take a look at @types if you're updating to Typescript 2 :)

On Sun, Sep 18, 2016, 12:56 Mark Pieszak notifications@github.com wrote:

Thank you @stephtr https://github.com/stephtr I'll take a look! I
wasn't sure if we would want to use typescript@next since it cold e very
unstable though?

One of the other main problems is that the store API in the one project is
very outdated model (based on an old tsd commit). Hoping I can spend some
time with Steve to look through the ReactRedux changes needed.

Other than that, I was able to get all the other projects running without
errors luckily.


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#258 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABeg9FXMArS5aaCZmpDCayWTDxSYbfUrks5qrV8-gaJpZM4JjbwR
.

@MarkPieszak
Copy link
Contributor Author

Course! Already have them in here 😃

@stephtr
Copy link
Contributor

stephtr commented Sep 18, 2016

One change made in Redux 3.6 is Improve typings for compose function, therefore a type definition is needed for devToolsExtension.

Using only a few changes ReactReduxSpa runs using TypeScript 2.0 (typescript@next not necessary) and @types.

While experimenting I noticed that TypeScript seems to use the typing included in the redux-package and not from @types/redux.

types/custom-types.d.ts is not needed when including node_modules/@types/webpack/webpack-env.d.ts in tsconfig.json (see DefinitelyTyped #10578).

@MarkPieszak
Copy link
Contributor Author

Hoping to get back to this one later today so we can get this merged in there since TS2 finally shipped :)
@SteveSandersonMS maybe we can link up later to figure out those ReactRedux issues? Seems the API has changed quite a bit for compose and a few other things.

Copy link

@antmdvs antmdvs left a comment

Choose a reason for hiding this comment

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

Added comments for ReactReduxSpa

@@ -4,6 +4,7 @@ import { routerReducer } from 'react-router-redux';
import * as Store from './store';
import { typedToPlain } from 'redux-typed';


export default function configureStore(initialState?: Store.ApplicationState) {
// Build middleware. These are functions that can process the actions before they reach the store.
const thunk = (thunkModule as any).default; // Workaround for TypeScript not importing thunk module as expected
Copy link

@antmdvs antmdvs Sep 30, 2016

Choose a reason for hiding this comment

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

I don't think this workaround is needed anymore and I believe you can just:
import thunk from 'redux-thunk';

@@ -1,7 +1,18 @@
{
"name": "WebApplicationBasic",
"name": "react-redux-spa",
"version": "0.0.0",
"dependencies": {
Copy link

Choose a reason for hiding this comment

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

Not sure why I can't comment on line 40 (it's grayed out), but bump typescript to v2.
"typescript": "^2.0.3",

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great idea! Let me add that in there.

@stephtr
Copy link
Contributor

stephtr commented Sep 30, 2016

Which issues are still left (except redux-typed)? Because in my fork it is working (stephtr/JavaScriptServices).

@MarkPieszak
Copy link
Contributor Author

Closed in favor of #362

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

Successfully merging this pull request may close these issues.

None yet

6 participants