fix: temporarily make logger optional and move into createStorageCont…

[entuziaz]

Fixes #252

This PR updates createStorageContext so logger is no longer a required positional argument and is now an optional field on CreateStorageContextOptions. This implementation still supports the previous structure internally to avoid breaking changes.

Formerly:

createStorageContext(synapse, logger, options?)

Updated:

createStorageContext(synapse, {
  ...options,
  logger
})

On Breaking Change: Internal callers and tests have been updated accordingly.

[SgtPooki]

@entuziaz I don't think we need to go this far with preventing breaking changes at this point since we're still in alpha. Let's go ahead and migrate it fully and ensure that anywhere we call it is up to date.

[entuziaz]

@SgtPooki Okay, I understand. Thank you.

[entuziaz]

@SgtPooki I got failures in validate-ipni-advertisement tests that seem unrelated to the createStorageContext change (e.g. lastActualMultiaddrs.intersection is not a function).

Should I look into the failing tests and see if I could fix them, or should I keep this PR focused on the logger-options issue at hand?

[SgtPooki]

@entuziaz lastActualMultiaddrs.intersection not a function seems to imply you're running an old nodejs version?

[entuziaz]

@entuziaz lastActualMultiaddrs.intersection not a function seems to imply you're running an old nodejs version?

This is interesting! Yeah, I had downgraded to Node.js v20 just a day ago when I pulled upstream updates, and my npm run build wouldn't run. Now that you pointed that out and I upgraded back, my build and tests are all running fine.

[SgtPooki]

@entuziaz it looks like the code in /upload-action also needs fixed to use the new function signature.

[entuziaz]

@entuziaz it looks like the code in /upload-action also needs fixed to use the new function signature.

Yeah, I didn't catch it from running npm run test:unit until I pushed and saw the CI test. Running full tests npm run test brought it out.

[SgtPooki]

a few minor changes still required.. thanks for your patience here

[entuziaz]

@SgtPooki, kindly check the updates.

[Copilot]

Pull request overview

This PR refactors the createStorageContext function to make the logger parameter optional by moving it from a required positional argument to an optional property within the CreateStorageContextOptions object. This change improves API flexibility and consistency.

Key Changes:

• Moved logger from second positional parameter to optional field in options object
• Updated all function calls throughout the codebase to use the new signature
• Added optional chaining for all logger method calls to handle undefined logger gracefully

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/core/synapse/index.ts	Updated CreateStorageContextOptions interface to include optional logger field; modified createStorageContext function signature and implementation to use optional chaining for logger calls
src/import/import.ts	Updated createStorageContext call to pass logger as part of options object; changed type annotation from Parameters<typeof createStorageContext>[2] to CreateStorageContextOptions
src/add/add.ts	Updated createStorageContext call to pass logger as part of options object; changed type annotation to use CreateStorageContextOptions
upload-action/src/filecoin.js	Updated createStorageContext call to include logger in the options object
src/test/unit/import.test.ts	Updated test expectations to verify logger is passed as part of options object rather than as separate parameter
src/test/unit/add.test.ts	Updated test expectations to match new function signature with logger in options
src/test/unit/dataset-management.test.ts	Updated all test calls to createStorageContext to pass logger as part of options object

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.