feat: add ts types with aegir #131
Conversation
hugomrdias
requested review from
vmx,
Gozala and
achingbrain
and removed request for
vmx and
Gozala
There was a problem hiding this comment.
Can you please remove the unrelated things like adding a GitHub issue template from this PR?
It also removes the withIs, which is a (potentially) huge breaking change. I'd prefer if we keep that in for compatibility. I don't think it makes sense to remove it as it will be replaced by multiformats/cid anyway.
@vmx |
|
@vmx im just trying to keep these repos consistent with each other and has you said we might move to the new multiformats soon, so theres no point in spending too much time in this and thats why i just added everything in. |
|
@Gozala @achingbrain @hugomrdias So we do a new major release of js-cid for adding TypeScript types? If there's agreement that's fine with me. @hugomrdias Though next time I'd prefer having things like adding issue templates in a separate commit as it's totally unrelated. |
|
@vmx nothing is actually a breaking change it behaves exactly as before, just a little bit safer now |
|
@Gozala @achingbrain @hugomrdias: oh sorry I missed that line: https://github.com/multiformats/js-cid/pull/131/files#diff-bfe9874d239014961b1ae4e89875a6155667db834a410aaaa2ebe3cf89820556R17 Which add the symbol which |
this is a best effort approach since multiformats/multiformats will probably superseed this repo soon.
|
I've removed the issue templates, I don't want them in this repo, I think it increases the burden of reporting a bug without having too much benefit (we also don't get that many bug reports here). |
The changes in #131 broke the tests of IPFS and probably quite a few other modules. This sort of thing used to work, now does not: ```js expect(ipfs.bitswap.unwant.calledWith(new CID(cidStr), defaultOptions)).to.be.true() ``` The reason it breaks is because internally `calledWith` does a `deepEqual` on the args which compares (among other things) the properties of the passed objects. We used to use `Object.defineProperty` to create cached versions of expensive to calculate fields which makes fields non-enumerable by default so they are skipped during the `deepEqual` check. different fields depending on how which constructor branch was hit or worse, if the instances properties have been accessed.
The changes in #131 broke the tests of IPFS and probably quite a few other modules. This sort of thing used to work, now does not: ```js expect(ipfs.bitswap.unwant.calledWith(new CID(cidStr), defaultOptions)).to.be.true() ``` The reason it breaks is because internally `calledWith` does a `deepEqual` on the args which compares (among other things) the properties of the passed objects. We used to use `Object.defineProperty` to create cached versions of expensive to calculate fields which makes fields non-enumerable by default so they are skipped during the `deepEqual` check. Now we just set the fields on the object which means instances have different fields depending on which constructor branch was hit or worse, if the instances properties have been accessed.
The changes in #131 broke the tests of IPFS and probably quite a few other modules. This sort of thing used to work, now does not: ```js expect(ipfs.bitswap.unwant.calledWith(new CID(cidStr), defaultOptions)).to.be.true() ``` The reason it breaks is because internally `calledWith` does a `deepEqual` on the args which compares (among other things) the properties of the passed objects. We used to use `Object.defineProperty` to create cached versions of expensive to calculate fields which makes fields non-enumerable by default so they are skipped during the `deepEqual` check. Now we just set the fields on the object which means instances have different fields depending on which constructor branch was hit or worse, if the instances properties have been accessed.
Fixes regression introduced by #131 due to incorrect type mappings
Fixes regression introduced by #131 due to incorrect type mappings
this is a best effort approach since multiformats/multiformats will probably superseed this repo soon.