Skip to content

fix(core): resolve TS2308 duplicate exports so @proof/core compiles under TS5#57

Open
devYRPauli wants to merge 1 commit into
EveryInc:mainfrom
devYRPauli:fix/doc-core-ts2308
Open

fix(core): resolve TS2308 duplicate exports so @proof/core compiles under TS5#57
devYRPauli wants to merge 1 commit into
EveryInc:mainfrom
devYRPauli:fix/doc-core-ts2308

Conversation

@devYRPauli

Copy link
Copy Markdown

Problem

The @proof/core barrel (packages/doc-core/src/index.ts) re-exports both
src/formats/marks.ts and src/formats/provenance-sidecar.ts with export *.
Both modules export the same three names — createComment,
getUnresolvedComments, and CommentReply — so under TypeScript 5 the barrel
fails to compile:

packages/doc-core/src/index.ts(2,1): error TS2308: Module
'../../../src/formats/marks.js' has already exported a member named
'createComment'. Consider explicitly re-exporting to resolve the ambiguity.

(same error for getUnresolvedComments and CommentReply). The result is that
@proof/core can't be imported from any consumer compiled under modern,
strict TypeScript.

Fix

Replace the export * from provenance-sidecar with explicit named
re-exports of its 21 non-colliding symbols (10 types + 11 values), letting
marks.ts own the three shared names. The marks-based API supersedes the
legacy comment helpers, so marks.ts is the natural owner.

No other public export changes — every previously-exported name remains
exported except the three duplicates, which are still available via marks.

Verification

  • Before: tsc --noEmit on a consumer of @proof/core emits the three TS2308
    errors above.
  • After: compiles clean; all of provenance-sidecar's non-colliding exports
    remain importable from @proof/core.

Question for maintainers

I let marks win the three colliding names. If you'd prefer
provenance-sidecar to own them instead, I'm happy to flip the direction —
just let me know.

The @proof/core barrel re-exports both formats/marks and
formats/provenance-sidecar with `export *`. Both modules export
`createComment`, `getUnresolvedComments`, and `CommentReply`, so under
TypeScript 5 the barrel fails to compile:

  src/index.ts: error TS2308: Module './formats/marks.js' has already
  exported a member named 'createComment'. Consider explicitly
  re-exporting to resolve the ambiguity.

(same for getUnresolvedComments and CommentReply). This makes @proof/core
unimportable from any consumer compiled under modern TS.

Replace the `export *` from provenance-sidecar with explicit named
re-exports of its 21 non-colliding symbols, letting marks.js own the three
shared names (the marks-based API supersedes the legacy comment helpers).
No other public export changes.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a914e83320

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +3 to +4
// createComment, getUnresolvedComments, and CommentReply are intentionally omitted
// here because marks.js owns those names in the new marks-based API.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Keep sidecar comment creation paired with addComment

For consumers that still use the legacy sidecar helpers from @proof/core, this omission makes createComment resolve to the marks API while addComment/ProvenanceData still come from provenance-sidecar. In the common flow addComment(provenance, createComment(selector, text, author)), TypeScript now rejects the Mark returned by the marks helper, and JavaScript callers would append a mark-shaped object to provenance.comments instead of a sidecar Comment, corrupting the legacy sidecar data. Consider preserving the sidecar helper under a non-colliding export or otherwise keeping the sidecar helper set internally consistent.

Useful? React with 👍 / 👎.

@devYRPauli

Copy link
Copy Markdown
Author

Thanks - that's a fair callout. The two createComment signatures do differ:

  • marks.js: createComment(quote, by, text, threadId?, range?, meta?) -> Mark
  • provenance-sidecar.ts: createComment(selector, text, author) -> Comment

and with this barrel the bare createComment / getUnresolvedComments / CommentReply names now resolve to the marks API.

Context for the choice: before this PR the barrel did export * from both modules, so those three names collided and TS2308 made @proof/core completely unimportable - no consumer could use the package at all. The goal here is just to make it compile again, and I let marks.js win the bare names since that is the forward (marks-based) API.

The downside is exactly what you flagged: a legacy sidecar caller writing addComment(prov, createComment(selector, text, author)) no longer type-checks through @proof/core. I'd rather not pick the public semantics unilaterally, so two options depending on the intended direction:

  1. Keep marks owning the bare names (current PR) and additionally re-export the sidecar helpers under non-colliding aliases (e.g. createComment as createSidecarComment, getUnresolvedComments as getUnresolvedSidecarComments) so legacy consumers still have a path.
  2. Flip it so the sidecar helpers keep the bare names and the marks versions get aliased.

Happy to push whichever you prefer - just let me know which name should own createComment and I'll update the PR.

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.

1 participant