make FileKeyInfo extensible for compatibility with TypeScript #273
Conversation
|
Should this be a more generic class such that other types of |
|
I'm in favor of that. About to push a change to the PR ... let me know what you think |
|
@cjbarth I just pushed up changes to my branch, let me know what you think! |
|
Those are some nice changes. I need a little more time to compare them to #249 and related code over at These issues have been open a while and I would really appreciate your help to wrangle them all together and get the functionality landed. Any help or comments you provide would be most welcome. |
|
Good morning @cjbarth ! I'm currently limited to Monday and Friday nights for development, though I may find occasions between to participate in discussions. Would you and @LoneRifle be interested in scheduling a virtual meeting to discuss the project roadmap? I envision an informal grooming session, of sorts, so we can tear through the high priority issues and get ourselves on the same page. Perhaps this way, we can step through the reported issues and formulate a plan of attack. |
|
@cjbarth & @LoneRifle - I do see why these issues are all related but I think the reality is that if you want to wrangle all the outstanding issues, then we're going to need a new major version number and put some breaking changes into it (that's fair play for a major version change). I'm putting together a proposal to address those issues for the time being and will share once I have something useful to work with. Either way, I propose merging this PR as a quick victory for the current major version. It doesn't block any solutions for the other issues, and solves the extensibility issue as well as providing a string-based implementation (issue #243 ). At its best, it can be used as a valid step towards addressing the other issues if we choose not to pursue the breaking-changes route. At it's worst, it's a tidy solution for the current version if we do choose to pursue the breaking-changes route. |
|
I don't mind. I think we also agreed that we want to do a major breaking version with package rename anyway, since xml-crypto has moved to @node-saml. The main blocker would be the lack of types from DefinitelyTyped, but this can be easily addressed by moving the types into this repo. I realize there is a fair bit of yak-shaving to do, so @djaqua if we need to move quickly, let me know, and I'll strive to find some time to look into this. cc @cjbarth |
|
@LoneRifle I've actually been in contact with the maintainer of the Definitely Typed type definitions for this project (there's a bug I volunteered to fix) and once I joined you and @cjbarth on this project I actually asked him to hold off on the fix until the dust settles here. I don't mind being a co-maintainer or liaison between this project and the type definitions project - I would be glad to help coordinate the maintenance and ensure continuity between the projects 🤷♂️ I thought I was the only person still using the expression "yak-shaving" lol
|
The type definitions for this awesome library require that the value of
SignedXML.keyInfoProviderbe an instance ofFileKeyInfo, but the methods on FileKeyInfo belong to the instance itself and not the prototype. This issue is only strictly a problem for TypeScript, but this change has a beneficial affect (in terms of reuse) on NodeJS clients, as well, and this is a non-breaking change.Also adding some documentation to the methods to clear up some of the confusion. Feedback is welcome!
Closes #272