Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for nullable types, map any as default type : fix #19, fix #31 #33

Merged
merged 1 commit into from
Jan 16, 2017
Merged

Add support for nullable types, map any as default type : fix #19, fix #31 #33

merged 1 commit into from
Jan 16, 2017

Conversation

abenhamdine
Copy link
Contributor

@abenhamdine abenhamdine commented Jan 15, 2017

Add support for nullable types.
For this, refactor mapTableDefinitionToType() : values in tableDefinition are now objects which store several properties, currently : udtName, nullable, tsType.
It allowed me to store nullable property, and it brings some extensibility to handle future properties for columns (eg comments).

I did not put this behind a flag as initially considered, because I don't see any good reason to lost information about nullable type. If a user don't care about checking null types, he would necessarily have disabled --strictNullChecks so this won't disturb him.

Considering this brings support for nullable types, I had to enable strictNullChecks and then I ran into null type error in src/index.ts L37 so I also made some changes here.

Also, as of now, SQL types which don't have defined mapping types will be converted to any type.

NB : this PR is against master, there will be merge conflicts with #32

NB2 : once again, I was unable to make diff test succeed on my PC, although test/artifacts/osm.ts and test/example/osm.ts are identical in VS Code (perhaps a pb related to carriage return encoding ?)

Add support of nullable types.
For this, refactor mapTableDefinitionToType() : values in
tableDefinition are now objects wich store several properties, currently
: udtName, nullable, tsType. It brings some extensibility to handle
future properties for columns.
@abenhamdine
Copy link
Contributor Author

abenhamdine commented Jan 15, 2017

ah CI failed because of 2 lines exceeding 120 characters, I forgot I have disabled this tslint rule on my PC 😞
@xiamx Does this prevent you to review/merge this one ?

@xiamx
Copy link
Contributor

xiamx commented Jan 16, 2017

@abenhamdine no problem :) I'll disable lint check on line length.. With type declaration on function arguments, it's very easy to go beyond the line limit

@xiamx xiamx merged commit a616275 into SweetIQ:master Jan 16, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants