to typescript #89
to typescript #89
Conversation
stefanpenner
force-pushed
the
typescript
branch
5 times, most recently
from
b9d1eee to
b9fd7cd
Compare
lib/index.ts
Outdated
| } | ||
| }; | ||
|
|
||
| class FSTree { |
There was a problem hiding this comment.
Consider making FSTree and BasicEntry having relativePath and isDirectory, and have a DefaultEntry interface match what defaultIsEqual uses. calculatePatch can further narrow equal in a generic to make the IsEqual type or the default if no isEqual function is passed in.
There was a problem hiding this comment.
Thanks, I got your example. I'll work on it after my next meeting.
stefanpenner
force-pushed
the
typescript
branch
from
b9fd7cd to
fbc145f
Compare
lib/index.ts
Outdated
| const inputPath = path.join(input, relativePath); | ||
| const outputPath = path.join(output, relativePath); | ||
|
|
||
| const delegateType = typeof delegate[methodName]; |
| } else if (entryA.mtime === undefined || entryB.mtime === undefined) { | ||
| equal = false; | ||
| } else if (+entryA.mtime === +entryB.mtime) { | ||
| equal = true; |
There was a problem hiding this comment.
I'd think you'd want to early return in these equal = true; cases.
There was a problem hiding this comment.
Ya, i toyed with several variations. The current ends up with the one i enjoyed the most.
stefanpenner
force-pushed
the
typescript
branch
4 times, most recently
from
aa12fec to
764a6d1
Compare
| j++; | ||
| additions.push(addOperation(y)); | ||
| } else { | ||
| if (!isEqual(x, y)) { |
There was a problem hiding this comment.
I'm actually not super sure how Typescript thinks this is valid yet. I'll have to do more investigation.
Basically, if T,K do not extend from or implement DefaultEntry, and isEqual is the defaultIsEqual. Then I think we should have a type mismatch...
There was a problem hiding this comment.
Because defaultIsEqual satisfies the a: T, b: K but it requires a: K, b: K.
lib/index.ts
Outdated
| } | ||
| }; | ||
|
|
||
| class FSTree<T extends BaseEntry> { |
There was a problem hiding this comment.
You might consider making the default entry type the default when you don't provide the generic type. class FSTree<T extends BaseEntry = DefaultEntry>
There was a problem hiding this comment.
I did not realize this was a thing. Cool!
stefanpenner
force-pushed
the
typescript
branch
from
764a6d1 to
cc23774
Compare
stefanpenner
force-pushed
the
typescript
branch
from
cc23774 to
d7d0bd8
Compare
|
I'm using this branch, because the currently released version doesn't play well with the latest walk-sync's types. |
|
@ef4 as a consumer any feedback? |
|
I think it's a breaking change for people like me who had previously written their own local types for it. But it was a pretty painless update, our types differed only cosmetically. Also, I found one significant bug. Though it's not really a bug in fs-tree-diff, it's a bug in ember-cli: new FSTree.fromEntries([]);That used to actually work, but now it's an exception. |
|
|
|
@ef4 submitted a fix ember-cli/ember-cli#8479 I'll release this as a major bump, so we can move forward and not break folks |
|
released as v2.0.0 |
|
@Turbo87 i even added a changelog entry for you |
|
😂 thanks! I'm curious why it's a major version bump though. Did the APIs change? nevermind, just saw the PR description |
|
Why is fromEntries([]) supposed to Error? What if you use walkSync on an empty directory? |
|
Nevermind I see it’s only with ‘new’ |
cc @mike-north / @krisselden r?
Although from CJS this isn't a breaking change, Typescript consumers it will be as it this project (to maintain CJS compat) must be required with
import FSTree = require('fs-tree-diff');