Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove circular dependency causing problems in lib consumer #598

Merged
merged 2 commits into from
Nov 6, 2023

Conversation

diehuxx
Copy link

@diehuxx diehuxx commented Nov 6, 2023

We have some circular dependencies in dwn-sdk-js that we should consider removing/resolving. One circular dependency in particular is causing issues in consumers of dwn-sdk-js.

In this PR dwn-sql-store, we are getting the following error upon running tests:

ReferenceError: Cannot access 'ed25519' before initialization
at (/home/runner/work/dwn-sql-store/dwn-sql-store/node_modules/@tbd54566975/dwn-sdk-js/src/jose/algorithms/signing/signature-algorithms.ts:8:17)
at ModuleJob.run (node:internal/modules/esm/module_job:217:25)
at async ModuleLoader.import (node:internal/modules/esm/loader:316:24)
at async importModuleDynamicallyWrapper (node:internal/vm/module:431:15)
at async formattedImport (/home/runner/work/dwn-sql-store/dwn-sql-store/node_modules/mocha/lib/nodejs/esm-utils.js:9:14)
at async exports.requireOrImport (/home/runner/work/dwn-sql-store/dwn-sql-store/node_modules/mocha/lib/nodejs/esm-utils.js:42:28)
at async exports.loadFilesAsync (/home/runner/work/dwn-sql-store/dwn-sql-store/node_modules/mocha/lib/nodejs/esm-utils.js:100:20)
at async singleRun (/home/runner/work/dwn-sql-store/dwn-sql-store/node_modules/mocha/lib/cli/run-helpers.js:125:3)
at async exports.handler (/home/runner/work/dwn-sql-store/dwn-sql-store/node_modules/mocha/lib/cli/run.js:370:5)

This culprit is importing index.js in ed25519.ts in this PR: https://github.com/TBD54566975/dwn-sdk-js/pull/591/files#diff-1b59382673e9e3dc1c3ec5cc4aebccb275379aa957877a6c0bdf8a6d2cadfcc0R5

Importing directly from dwn-error.ts instead of index.js fixed the error in dwn-sql-store.

Copy link

codesandbox bot commented Nov 6, 2023

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

@diehuxx diehuxx force-pushed the diehuxx/dwn-error-circ-dep branch from 5276505 to 7c4b0e5 Compare November 6, 2023 21:23
thehenrytsai
thehenrytsai previously approved these changes Nov 6, 2023
LiranCohen
LiranCohen previously approved these changes Nov 6, 2023
@diehuxx diehuxx dismissed stale reviews from LiranCohen and thehenrytsai via 1330bea November 6, 2023 21:31
@codecov-commenter
Copy link

codecov-commenter commented Nov 6, 2023

Codecov Report

Merging #598 (1330bea) into main (9a4edc6) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main     #598   +/-   ##
=======================================
  Coverage   97.82%   97.82%           
=======================================
  Files          66       66           
  Lines        8373     8373           
  Branches     1200     1200           
=======================================
  Hits         8191     8191           
  Misses        176      176           
  Partials        6        6           
Files Coverage Δ
src/jose/algorithms/signing/ed25519.ts 95.08% <100.00%> (ø)

@diehuxx diehuxx merged commit 141ef49 into main Nov 6, 2023
4 checks passed
@thehenrytsai thehenrytsai deleted the diehuxx/dwn-error-circ-dep branch April 8, 2024 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants