prismake / typegql

Create GraphQL schema with TypeScript classes.
https://prismake.github.io/typegql/
MIT License
423 stars 21 forks source link

Fix type declerations #24

Closed andrew-w-ross closed 6 years ago

andrew-w-ross commented 6 years ago

Added '~' path to refer to ./src in tsconfig Changed jest module resolution to understand that ~ referes to ./src Added ttypescript and @zerollup/ts-transform-paths to transform paths Changed build:types task to use ttsc with transform. fixes prismake/typegql/issues/#23

codecov[bot] commented 6 years ago

Codecov Report

Merging #24 into master will decrease coverage by 0.03%. The diff coverage is 86.48%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #24      +/-   ##
==========================================
- Coverage   86.92%   86.88%   -0.04%     
==========================================
  Files          62       62              
  Lines        1086     1083       -3     
  Branches      188      188              
==========================================
- Hits          944      941       -3     
  Misses        141      141              
  Partials        1        1
Impacted Files Coverage Δ
src/domains/inputField/compiler/index.ts 91.66% <ø> (ø) :arrow_up:
src/domains/field/compiler/services.ts 88.88% <ø> (ø) :arrow_up:
src/domains/objectType/compiler/objectType.ts 83.33% <0%> (ø) :arrow_up:
src/domains/inputObjectType/compiler/objectType.ts 76.66% <0%> (ø) :arrow_up:
src/domains/field/registry.ts 100% <100%> (ø) :arrow_up:
src/domains/inputField/compiler/fieldType.ts 64.28% <100%> (ø) :arrow_up:
src/domains/hooks/registry.ts 86.66% <100%> (ø) :arrow_up:
src/domains/arg/registry.ts 100% <100%> (ø) :arrow_up:
src/domains/schema/registry.ts 100% <100%> (ø) :arrow_up:
src/domains/field/compiler/resolver.ts 91.02% <100%> (ø) :arrow_up:
... and 22 more

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f23daef...6195f2a. Read the comment docs.

pie6k commented 6 years ago

Hey, thanks a lot.

While I'll propably agree anyway - but could you describe motivation behind this change a bit?

Also, in many places you're using tabs instead of spaces in code formatting. Could you try to follow used convention?

Thanks again

andrew-w-ross commented 6 years ago

Hey @pie6k,

If you look at #23 the emitted definitions aren't usable when imported to another project. This is because of this issue.

To fix that I added a transform to fix the emitted declarations and make them relative.. I described that build by adding the ttsconfig.json and changing the build:emit task to use that ttsc with that config.

As for the path change, I used ~ to refer to root. It was hard to distinguish when if you are importing a package or project files. When reading the code now it's a lot easier to see that ~/domains refers to a the src/domains vs domains that looks like a package. Also I'm not entirely sure the path transform could work without it.

andrew-w-ross commented 6 years ago

@pie6k I've sorted out the tabs to spaces issues. Anything else needed to get this pr merged in?

pie6k commented 6 years ago

I'll make sure everything works and try to merge it tomorrow ;)

andrew-w-ross commented 6 years ago

Thanks

pie6k commented 6 years ago

@andrew-w-ross hey, let me know if you'd be able to fix those tabs thing ;)

andrew-w-ross commented 6 years ago

@pie6k That should be all of them.

andrew-w-ross commented 6 years ago

@pie6k I've just double checked using several other text editors and \t is not to be found.