Repository navigation
refactor: make provider imports go one way, so no file imports a file that imports it back - #25
Merged
Merged
Conversation
… that imports it back Each provider's API service (GitHubApi, LinearApi, SlackApi) returned objects whose methods call the API service back: postIssueComment returned a GitHubIssueComment, whose addReaction() calls GitHubApi. So the API service files and the objects files imported each other. In Linear and Slack that crashed at startup depending on which file loaded first, and was patched over with second tags sharing the services' keys (LinearApiHandle, SlackApiHandle). Now each provider's files fall into three groups, and each imports only from the ones before it: - Plain data: refs, ids, and the plain records the API returns. No methods. - The API service: takes and returns plain data only, and never imports the objects. - Callback objects (GitHubIssue, GitHubIssueComment, LinearFile, SlackMessage, ...): wrap plain data; their methods call the API service and wrap what it returns. - The API services now return records (`GitHubIssueCommentRecord`, `LinearCommentRecord`, `SlackMessageRecord`, ...) instead of objects with methods. Callback code is unchanged: `issue.postComment()`, `thread.listMessages()`, and `file.download()` still return objects. - Objects with exactly a record's fields are built with `X.make(record)`; objects that hold other objects have a static `X.fromRecord(record)` (`LinearComment`, `SlackMessage`). - LinearApiHandle and SlackApiHandle are deleted; the objects use the real tags. - Slack's SlackModels.ts held both plain data and objects; SlackMessage and SlackFile move to their own files. This also fixes a startup crash that was already on main (importing SlackModels first). - Linear's plain file and resource data move to LinearFileModels.ts and LinearResourceRecords.ts; GitHub's plain data the API returns moves to GitHubModels.ts. - The Alchemy Cloudflare example's helpers that read comments from GitHubApi now take records. No package's source has an import cycle, and every provider module loads on its own. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> HumanLayer-Session: https://app.dev.codelayer.gg/sessions/01a1418a-45ea-7a36-87bf-d2e8c4e465fe
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Each provider's API service returned objects whose methods call the API service back. For example,
GitHubApi.postIssueCommentreturned aGitHubIssueComment, and that comment'saddReaction()callsGitHubApi. So the API service file and the objects file each imported the other.In Linear and Slack this crashed at startup, depending on which file loaded first. #24 patched over it with second tags that share the service's key (
LinearApiHandle,SlackApiHandle). This PR removes that workaround and fixes the cause.What changed
Each provider's files now fall into three groups, and each group imports only from the groups above it. These are groups of files, not Effect
Layers; how services are built and provided doesn't change.GitHubApi,LinearApi,SlackApi): takes and returns plain data only, and never imports the objects.GitHubIssue,GitHubIssueComment,LinearFile,SlackMessage, …): wrap plain data. Their methods call the API service and wrap what it returns.GitHubIssueCommentRecord,LinearCommentRecordandSlackMessageRecord, instead of objects with methods.issue.postComment(),thread.listMessages()andfile.download()still return objects.X.make(record). Objects that hold other objects have a staticX.fromRecord(record)(LinearComment,SlackMessage).LinearApiHandleandSlackApiHandle.SlackModels.tsheld both plain data and objects, soSlackMessageandSlackFilemove to their own files. This also fixes a startup crash that was already onmain: importingSlackModelsfirst.LinearFileModels.tsandLinearResourceRecords.ts.GitHubModels.ts.GitHubApinow take the record types. The tests' fake APIs return records.Breaking changes
Code that calls an API service directly gets records back:
listIssueCommentsandlistPullRequestCommentsreturnGitHubIssueCommentRecords.listPullRequestReviewCommentsreturnsGitHubReviewCommentRecords.postIssueCommentandpostPullRequestCommentreturnGitHubIssueCommentRecord.postPullRequestReviewCommentandreplyToReviewCommentreturnGitHubReviewCommentRecord.updateCommentreturnsGitHubCommentRecord.listCheckRunsForRefreturnsGitHubCheckRunRecords.resolveActionsJobreturnsGitHubActionsJobRecord | null.listIssueCommentsreturnsLinearCommentRecord[].listIssueAttachmentsreturnsLinearIssueAttachmentRecord[].createCommentandupdateCommentreturnLinearCommentRecord.createReactionreturnsLinearReactionRecord.createAttachment,updateAttachmentanduploadAttachmentreturnLinearIssueAttachmentRecord.uploadFilereturnsLinearFileRecord.listThreadMessagesandlistChannelMessagesBeforeThreadreturnSlackMessageRecords.getMessagereturnsSlackMessageRecord.postToThread,postToChannelandstreamreturnSlackSentMessageRecord.uploadFileToChannelanduploadFileToThreadreturnSlackFileRecord.LinearFileRef(now inLinearFileModels) andLinearFileError(now inLinearApiContract). All are still exported from each package's root.Checks
bun run check(lint, typecheck, typed-lint) andbun run format: pass.bun run test: 742 tests pass.srcor in the example.🤖 Generated with Claude Code