veramo icon indicating copy to clipboard operation
veramo copied to clipboard

feat(credential-ld): Added `Ed25519Signature2020` & `JsonWebSignature2020` experimental support

Open Eengineer1 opened this issue 1 year ago • 3 comments

Linked issue & detailed approach #1003 .

Eengineer1 avatar Oct 12 '22 10:10 Eengineer1

Thanks for adding this. It looks like these new packages or rather their dependencies are causing conflicts in some environments.

We ran into lots of such issues with @transmute and @digitalbazaar dependencies while trying to add JSON-LD support, and I don't know of a solution that works in all cases.

mirceanis avatar Oct 14 '22 08:10 mirceanis

@mirceanis Thanks for mentioning. I can recall from our early chat about dependency issues.

What's included as well this PR:

  • Bump @types/eslint, 8.4.3 -> 8.4.6.
  • Bump @types/eslint-scope, 3.7.3 -> 3.7.4.
  • Added as direct dependency in test-react-app to not clash with lower expected versions.
  • Added node version compatible < v16.x, [email protected] for multibase keys.

I'm pushing a fix on @digitalcredentials/vc <> @transmute/json-web-signature compatibility later today.

Questions:

  • Are we open to exploring bumps, as long as the tests work as existing?
  • To what extent are we open on refactoring tests to comply to bumps?

Eengineer1 avatar Oct 14 '22 11:10 Eengineer1

Questions:

  • Are we open to exploring bumps, as long as the tests work as existing?

What do you mean by bumps? version bumps? Then yes. We try to keep up-to-date with all dependencies. We use renovate-bot to automatically bump dependencies that are safe.

  • To what extent are we open on refactoring tests to comply to bumps?

Tests should reflect real-world scenarios as close as possible. If we can't get our tests to run it's very likely that some user will face the same issues, so we are not going to include changes that we can't handle ourselves. Refactoring tests is acceptable, as long as it doesn't hide any issues coming from dependencies.

We have a number of constraints that we impose on the packages in this repository:

  • they should all work in all 3 environments: nodejs, web, react-native
  • it's ok to also need workarounds for some environments as long as we know what the workarounds are
  • Veramo plugin method arguments and results should be serializable to JSON (this allows us to automatically compute the OpenAPI schemas)
  • plugins must not conflict with each-other; a user should be able to create a Veramo instance with all available plugins.

If we can't enforce these constraints on a new plugin then it should not be in this repository.

mirceanis avatar Oct 14 '22 11:10 mirceanis

Codecov Report

Merging #1030 (f2aa85a) into next (125bf42) will decrease coverage by 0.57%. The diff coverage is 76.71%.

Additional details and impacted files
@@            Coverage Diff             @@
##             next    #1030      +/-   ##
==========================================
- Coverage   80.25%   79.67%   -0.58%     
==========================================
  Files         118      127       +9     
  Lines        4056     4610     +554     
  Branches      875      993     +118     
==========================================
+ Hits         3255     3673     +418     
- Misses        798      933     +135     
- Partials        3        4       +1     

codecov[bot] avatar Oct 21 '22 18:10 codecov[bot]

@mirceanis Hey, this is ready for your review.

Kudos to the Transmute team on helping out untangle the non-polyfilled dependency knot upstream.

Eengineer1 avatar Oct 21 '22 19:10 Eengineer1

@mirceanis Hey, this is ready for your review.

This looks good. Can you try to rebase to fix the conflicts?

mirceanis avatar Nov 17 '22 13:11 mirceanis

@mirceanis Hey, this is ready for your review.

This looks good. Can you try to rebase to fix the conflicts?

Resolved.

Eengineer1 avatar Nov 18 '22 12:11 Eengineer1