TypeScript declaration files #251

lutovich · 2017-06-18T22:08:44Z

Added TypeScript declaration files for all public driver APIs. These should make it easier to use driver in TypeScript projects.

PR is heavily based on #183. Thanks for contribution @SnappyCroissant and @pocesar.

Closed #77

Not ready for merge: all declaration files should be covered with tests. Currently on transaction.d.ts is.

lutovich · 2017-06-18T22:11:54Z

Hi @SnappyCroissant and @pocesar.
Could you guys please review this PR? I made some changes to your initial PR #183. Most significant changes include:

removed definitions for non-public classes and functions
used interfaces as much as possible to define API shapes
moved type declarations to a dedicated folder "types"
started adding tests

@oskarhane could you please review as well? and share your general feeling about this?

oskarhane · 2017-06-19T10:02:53Z

I'm positive, this is a good step.
As far as reviewing I put my trust in @pocesar and @SnappyCroissant 😃

pocesar · 2017-06-19T13:46:18Z

@lutovich looks good! I will eventually help with the generics (by using your own created interfaces to create safe code through the driver), but that's the cherry on top, the base definitions LGTM

EDIT: for example, when using subscribe, you'd want to have a subscribe<T>(observer: Observer<T>) so your records reflect whatever you pass in for the T generic, or a strongly typed generic for Result

lutovich · 2017-06-19T13:57:31Z

@pocesar thanks for the review!
Actually, I removed generics from Result, Record and Observer on purpose. One can get back a Record of mixed types so I did not quite understand why should it have a generic signature. For example this code is valid:

const result: Result = session.run("RETURN 42, "42", ["42"]");
const record: Record = result.records[0];
const value1: number = record.get(0);
const value2: string = record.get(1);
const value3: string[] = record.get(2);

Am I missing something?

pocesar · 2017-06-19T15:01:00Z

in TS you can have union types, like run<Type1 & Type2 & Type3>(), so you can actually have one that has them all, all you can use run<Type1 | Type2 | Type3>() then after checking for their types, you get the correct type in a scope, like:

tx.run<Partial<Type1 & Type2 & Type3>>().records[0].// has members of Type1, 2 and 3, but if you use |, you can refine the type by checking for it's members

besides writing less code and not having to rely on variables, it makes composition easier (even more when dealing with async / promises code), so when you're writing wrappers around neo4j driver, and consuming in your code, it becomes much more evident, since you don't need to check the code to understand what will come out of the function / method (talking consumers / team members here)

check Discriminated Unions in https://www.typescriptlang.org/docs/handbook/advanced-types.html

TimothyMischief · 2017-06-20T01:40:50Z

Everything within my understanding looks good. Discriminated Unions are a bit above my level of understanding but I'm educating myself now!

lutovich · 2017-06-20T07:52:14Z

Nice, thanks for review!
I need to read more about TS type system as well.
Will update PR with more tests before merging.

cojack · 2017-06-21T19:27:57Z

Im counting on you @lutovich

Covered All Externals I think. First time writing Type Definitions so anyone else feel free to take over and do a better job.

Removed a Comma that had wound up in a js file when I was first figuring everything out.

mv command left over can't use types as variables add some missing stuff, fix implicitAny errors can't use types for vars noImplicitAny and optional parameters type safety for session.run results needs typeof in const declaration for it to work result summary members unprotect members

there's no public accessors in records member

_fields is wrong, it should be an array of T

Commit moves all type script definition files from `lib` folder to a dedicated folder `types` located in the project root. This is cleaner because `lib` contains generated sources.

Changes include: * removed definitions for non-public classes and functions * used interfaces as much as possible to define API shapes * added super-interface `StatementRunner` for `Session` and `Transaction` to define signature of `#run()` is one place * added license headers * applied default IntelliJ code formatter

This commit adds a first simple test for transaction class TypeScript definitions. Actual test code is not executed, it is just compiled using the TypeScript compiler to verify correctness of types. Also added npm script to run TS definition tests `test-types`. More tests will be added in subsequent commits.

And updated corresponding npm script: `run-ts-declaration-tests`. It is now part of gulp's `test` task.

cojack · 2017-06-22T16:49:08Z

@lutovich please don't forget to bump version and release it at npm ❤️
Thanks in advance.

lutovich · 2017-06-22T16:52:16Z

@cojack these should be released with 1.4.0-beta01 early next week. Pre-release versions are available with next npm tag. I.e. npm install --save neo4j-driver@next.

lutovich · 2017-06-26T10:09:53Z

Pre-release version 1.4.0-beta01 is now available on npm, please feel free to try it out, give feedback and contribute.

cojack · 2017-06-26T13:57:55Z

@lutovich I had a problem with explicitly property type definition.
When I try something like that:

import * as Neo4j from 'neo4j-driver';
// stuff
private driver: Neo4j.v1.Driver;

Typescript said:

Error:(11, 18) TS2694:Namespace '"/home/cojack/Projects/neo4j-test/node_modules/neo4j-driver/types/v1/index"' has no exported member 'Driver'.

So I resigned with type annotation for this field, but I guess it should works.

lutovich · 2017-06-26T15:24:52Z

@cojack non-default declaration does not export Driver. Could you please try:

import Neo4j from 'neo4j-driver';
// stuff
private driver: Neo4j.Driver;

I'm not quite sure if we need to export all types from v1/index.d.ts. You could as well import Driver using import {Driver} from "neo4j-driver/types/v1/driver";. Some input here is very welcome.

cojack · 2017-06-26T18:00:00Z

@lutovich nope, it doesn't not work with your proposal. Anyway, doesn't have to 👍
Thanks for types 😃

lutovich mentioned this pull request Jun 18, 2017

Started Adding Type Definitions #183

Closed

cojack approved these changes Jun 22, 2017

View reviewed changes

TimothyMischief and others added 18 commits June 22, 2017 14:36

Started Adding Type Definitions

d181b04

Covered All Externals I think. First time writing Type Definitions so anyone else feel free to take over and do a better job.

Removed Comma

d4a4458

Removed a Comma that had wound up in a js file when I was first figuring everything out.

Update result.d.ts

c294ea2

there's no public accessors in records member

Update record.d.ts

2338692

_fields is wrong, it should be an array of T

export definitions (Driver and Session) and fix subscribe

d58f3d5

Move TS definition files to a dedicated folder

c901500

Commit moves all type script definition files from `lib` folder to a dedicated folder `types` located in the project root. This is cleaner because `lib` contains generated sources.

Test error and graph types TS declarations

7928552

Test record TS declarations

85a570a

Test integer TS declarations

5625f67

Test result TS declarations

804a3ea

Test result-summary TS declarations

3b1584f

Test session TS declarations

1ed5e0c

Test driver TS declarations

3b512a3

Test index TS declarations

61fde2c

Made gulp run TS declaration tests

0a52c19

And updated corresponding npm script: `run-ts-declaration-tests`. It is now part of gulp's `test` task.

lutovich force-pushed the type-definitions branch from 353380c to 0a52c19 Compare June 22, 2017 12:44

lutovich merged commit 68cbf8b into neo4j:1.4 Jun 22, 2017

lutovich deleted the type-definitions branch June 22, 2017 16:44

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TypeScript declaration files #251

TypeScript declaration files #251

lutovich commented Jun 18, 2017 •

edited

lutovich commented Jun 18, 2017 •

edited

oskarhane commented Jun 19, 2017

pocesar commented Jun 19, 2017 •

edited

lutovich commented Jun 19, 2017

pocesar commented Jun 19, 2017 •

edited

TimothyMischief commented Jun 20, 2017

lutovich commented Jun 20, 2017 •

edited

cojack commented Jun 21, 2017

cojack commented Jun 22, 2017

lutovich commented Jun 22, 2017

lutovich commented Jun 26, 2017

cojack commented Jun 26, 2017

lutovich commented Jun 26, 2017

cojack commented Jun 26, 2017

TypeScript declaration files #251

TypeScript declaration files #251

Conversation

lutovich commented Jun 18, 2017 • edited

lutovich commented Jun 18, 2017 • edited

oskarhane commented Jun 19, 2017

pocesar commented Jun 19, 2017 • edited

lutovich commented Jun 19, 2017

pocesar commented Jun 19, 2017 • edited

TimothyMischief commented Jun 20, 2017

lutovich commented Jun 20, 2017 • edited

cojack commented Jun 21, 2017

cojack commented Jun 22, 2017

lutovich commented Jun 22, 2017

lutovich commented Jun 26, 2017

cojack commented Jun 26, 2017

lutovich commented Jun 26, 2017

cojack commented Jun 26, 2017

lutovich commented Jun 18, 2017 •

edited

lutovich commented Jun 18, 2017 •

edited

pocesar commented Jun 19, 2017 •

edited

pocesar commented Jun 19, 2017 •

edited

lutovich commented Jun 20, 2017 •

edited