Repository navigation
refactor(lib): fully remove barrel imports from @pagespace/lib - #1087
Closed
2witstudios wants to merge 10 commits into
Closed
2witstudios wants to merge 10 commits into
2witstudios wants to merge 10 commits into
Conversation
Replace 12 flat export aliases in @pagespace/lib with paths that match the actual source layout: - ./secure-compare, ./device-auth-utils, ./broadcast-auth, ./verification-utils → ./auth/* - ./activity-tracker, ./ai-monitoring, ./ai-context-calculator → ./monitoring/* - ./logger-config, ./logger-database, ./logger-browser, ./logger → ./logging/* - ./api-utils → ./utils/api-utils Updates 114 import statements across apps/web, apps/realtime, and apps/processor. Both exports and typesVersions fields in package.json updated in lockstep. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The first pass only caught static `from '...'` imports. vi.mock(), vi.doMock(), await import(), and vitest.config alias maps also contained the old flat paths. Replace all remaining occurrences across apps/web, apps/realtime, and apps/processor. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
All three trackAuthEvent calls (web, desktop, iOS platforms) were passing the raw email address. maskEmail was already imported from @pagespace/lib/server and used in logger calls in the same file. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Migrates 533 files across apps/web, apps/processor, apps/realtime, and apps/control-plane from high-level barrel re-exports (server, auth, integrations, notifications) to direct subpath imports. Adds missing package.json exports/typesVersions entries for: - integrations/repositories/provider-repository - integrations/credentials/encrypt-credentials - integrations/oauth/oauth-handler - integrations/providers - integrations/converter/openapi Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…arrel removal After removing barrel imports from @pagespace/lib, test mocks targeting old barrel paths (e.g. @pagespace/lib/auth, @pagespace/lib/server) no longer intercept the source code which now imports from specific subpaths. Updates all affected test files to mock the exact subpath each source file imports from, and fixes residual broken mock patterns (e.g. `...actual` spread without vi.importActual) introduced by the migration script. Also fixes TypeScript typecheck error in admin-role-version.test.ts caused by the same broken pattern. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
Important Review skippedToo many files! This PR contains 300 files, which is 150 over the limit of 150. ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (300)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b1a84ae4e9
ℹ️ 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".
| @@ -0,0 +1 @@ | |||
| /Users/jono/production/PageSpace/packages/lib/dist | |||
There was a problem hiding this comment.
Do not commit packages/lib/dist as an absolute symlink to /Users/jono/...: that target only exists on one machine, so every other environment gets a broken @pagespace/lib package root/subpath resolution and downstream TypeScript/runtime failures when imports resolve through dist. Commiting this link makes builds non-portable across CI and other developers.
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
Fixed in 7131b27 — removed both symlinks from git tracking with git rm --cached. These are local pnpm worktree artifacts that were accidentally staged by git add -A in the migration commit. They are not in .gitignore as symlinks (only dist/ directories are ignored, not symlinks named dist).
| @@ -0,0 +1 @@ | |||
| /Users/jono/production/PageSpace/packages/db/dist | |||
There was a problem hiding this comment.
packages/db/dist is also introduced as an absolute symlink to /Users/jono/..., which is not valid outside the author’s workstation. Any workflow that resolves @pagespace/db via its package entry points will fail in other environments because the linked dist tree does not exist there.
Useful? React with 👍 / 👎.
Owner
Author
There was a problem hiding this comment.
Fixed in 7131b27 — same as packages/lib/dist, removed from git tracking.
packages/lib/dist and packages/db/dist were committed as absolute symlinks pointing to /Users/jono/... — valid only on the author's machine, breaking CI and all other environments. Removed from git tracking. The symlinks are local worktree artifacts created by pnpm workspace that must not be committed. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Adds escapeRegExp helper to both migration scripts to properly escape special characters in strings interpolated into RegExp constructors. Addresses CodeQL findings: - scripts/fix-wrong-imports.mjs:132 — Incomplete string escaping - scripts/migrate-barrel-imports.mjs:433 — Regular expression injection Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Prevents accidental re-commit of packages/lib/dist and packages/db/dist symlinks which are local pnpm workspace artifacts, not source files. The existing `dist/` pattern only ignores real directories, not symlinks. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Owner
Author
|
Closing in favor of a 6-PR series that stays under CodeRabbit's review limit and groups changes by logical subsystem:
All work from this PR is preserved in those branches. Merge order: #1088 first, then #1089–#1093 in any order. |
Owner
Author
|
Closing in favor of a 6-PR series grouped by logical subsystem:
All work from this PR is preserved in those branches. Merge order: #1088 first, then #1089–#1093 in any order. |
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
2 participants
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.
Summary
This PR completes the removal of all barrel re-export aggregators from
@pagespace/lib, replacing every high-level barrel import (@pagespace/lib,@pagespace/lib/server,@pagespace/lib/auth,@pagespace/lib/permissions,@pagespace/lib/integrations,@pagespace/lib/client-safe,@pagespace/lib/security,@pagespace/lib/content,@pagespace/lib/notifications) with direct subpath imports across all apps and test files.Why this matters:
#1077established real subdir paths in exports, the barrels became dead weight — this PR cleans them outWhat changed:
packages/lib/package.json: Removed barrel export aliases; all exports are now direct subpath entriesapps/web/src/**: ~250 source files + test files updated to use specific subpaths (e.g.@pagespace/lib/auth/session-serviceinstead of@pagespace/lib/auth)apps/realtime/src/**,apps/processor/src/**,apps/control-plane/src/**,apps/marketing/src/**: Same treatmentscripts/migrate-barrel-imports.mjs: Migration script used to automate the bulk of changesPre-existing test failures (not caused by this PR, pass in CI with test DB):
admin-role-version.test.ts— DB integration test, requires PostgreSQLgift-subscription/route.security.test.ts— DB integration test, requires PostgreSQLfetch-bridge.test.ts— network timeout, requires real networkpackages/lib rate-limit-cache.integration.test.ts— DB integration testTest plan
pnpm --filter web typecheckpasses with zero errorspnpm --filter web exec vitest run— 379 test files pass, 3 pre-existing DB-dependent failures unchangedpackages/lib vitest run— 152 test files pass, 1 pre-existing DB-dependent failure unchangedReviewer notes
Please use
/aidd:reviewto conduct a thorough review of this PR. The key things to verify:@pagespace/libroot,@pagespace/lib/server,@pagespace/lib/auth(as a barrel),@pagespace/lib/permissions(as a barrel), etc.vi.mockpath must exactly match what the source file under test imports; any mismatch means the mock silently does nothingimport { X } from '@pagespace/lib/barrel'→import { X } from '@pagespace/lib/specific/subpath'🤖 Generated with Claude Code