Skip to content
This repository has been archived by the owner on Sep 21, 2019. It is now read-only.

Update TypeScript to 2.6 #16

Merged
merged 8 commits into from
Jan 20, 2018
Merged

Update TypeScript to 2.6 #16

merged 8 commits into from
Jan 20, 2018

Conversation

vincentbel
Copy link
Contributor

I split this into multiple commits for easier reviewing. You can view commits one by one.

TypeScript will complain about:
"Type 'Node' is not assignable to type 'SourceFile'."
ts.NodeArray now is ReadonlyArray, helper functions should accept
ArrayLike arguments.
@@ -39,12 +39,12 @@ export const allTransforms = [
reactRemoveStaticPropTypesMemberTransformFactoryFactory,
];

export type TransformFactoryFactory = (typeChecker: ts.TypeChecker) => ts.TransformerFactory<ts.Node>;
export type TransformFactoryFactory = (typeChecker: ts.TypeChecker) => ts.TransformerFactory<ts.SourceFile>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

TypeScript will complain it if we use <ts.Node> here.

image

@@ -150,10 +121,12 @@ export function insertAfter<T>(collection: T[], afterItem: T, newItem: T) {
* @param beforeItem
* @param newItem
*/
export function insertBefore<T>(collection: T[], beforeItem: T, newItem: T) {
const index = collection.indexOf(beforeItem);
export function insertBefore<T>(collection: ArrayLike<T>, beforeItem: T, newItems: T | T[]) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need use ArrayLike type now because ts.NodeArray now is ReadonlyArray.

image


// export const has = some
Copy link
Contributor

Choose a reason for hiding this comment

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

please remove this


return false;
}
export { find };
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets use find imported from loads where it is needed

import * as kinds from './isKind';

export * from './isKind';
import { find, indexOf, slice } from 'lodash';
Copy link
Contributor

Choose a reason for hiding this comment

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

we're using namespace imports in this project

import * as _ from 'lodash'

) {
const allMembers = node.type.types
.map((type: ts.TypeLiteralNode) => type.members)
const allMembers = (node.type.types as ts.NodeArray<ts.TypeLiteralNode>)
Copy link
Contributor

Choose a reason for hiding this comment

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

Why we have to cast to ts.NodeArray<ts.TypeLiteralNode> here?

Copy link
Contributor Author

@vincentbel vincentbel Jan 20, 2018

Choose a reason for hiding this comment

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

Because TypeScript can't figure out the previous node.type.types.every(...), and still think the type of node.type.types is ts.NodeArray<ts.TypeNode>, rather than ts.NodeArray<ts.TypeLiteralNode>.

image

I can add a comment here.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok I think casting should be fine for now. I'll dig deeper into it soon.

/cc @DanielRosenwasser

@mohsen1
Copy link
Contributor

mohsen1 commented Jan 20, 2018

A few notes, otherwise looks good!

Thanks!

@mohsen1 mohsen1 merged commit aaf86ed into lyft:master Jan 20, 2018
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.

2 participants