dojo / core

:rocket: Dojo 2 - language helpers and utilities.
http://dojo.io
Other
213 stars 62 forks source link

Enhance `assign()` typings to allow different types of sources. #317

Closed lzhoucs closed 7 years ago

lzhoucs commented 7 years ago

Type: bug

The following has been addressed in the PR:

Description:

The updated typings for assign() is the same as in Typescript.

I looked at Partial but so far I haven't found a way to solve the problem with it.

Although this solution should resolve #291 , it is still based on the same intersection types we've been using, so due to the limitation of intersection types, there's still other typings issues. For example:

const result  = lang.assign({foo: 1}, {foo: 'foo'}); // result.foo becomes `string & number`
result.foo = 'bar'; // won't compile

And a workaround:

// workaround
type Foo = { foo: string };
const result: Foo  = lang.assign({foo: 1}, {foo: 'foo'}); // result.foo becomes `string`
result.foo = 'bar'; // works

Resolves #291

kitsonk commented 7 years ago

Merged this, because the CI is passing except for the api document generating trying to be generated on non-master branches... 😢

codecov[bot] commented 7 years ago

Codecov Report

Merging #317 into master will increase coverage by 0.36%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #317      +/-   ##
==========================================
+ Coverage   93.88%   94.25%   +0.36%     
==========================================
  Files          39       39              
  Lines        2144     2228      +84     
  Branches      408      452      +44     
==========================================
+ Hits         2013     2100      +87     
+ Misses         52       51       -1     
+ Partials       79       77       -2
Impacted Files Coverage Δ
src/lang.ts 100% <100%> (ø) :arrow_up:
src/request/providers/node.ts 92.49% <0%> (+4.45%) :arrow_up:

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 a77d9e7...226908d. Read the comment docs.

codecov[bot] commented 7 years ago

Codecov Report

Merging #317 into master will increase coverage by 0.36%. The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #317      +/-   ##
==========================================
+ Coverage   93.88%   94.25%   +0.36%     
==========================================
  Files          39       39              
  Lines        2144     2228      +84     
  Branches      408      452      +44     
==========================================
+ Hits         2013     2100      +87     
+ Misses         52       51       -1     
+ Partials       79       77       -2
Impacted Files Coverage Δ
src/lang.ts 100% <100%> (ø) :arrow_up:
src/request/providers/node.ts 92.49% <0%> (+4.45%) :arrow_up:

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 a77d9e7...226908d. Read the comment docs.