Skip to content

chore(lint): import Effect modules from their subpaths - #16326

Closed
esthor wants to merge 4 commits into
pingdotgg:mainfrom
esthor:lint/effect-subpath-imports
Closed

esthor wants to merge 4 commits into
pingdotgg:mainfrom
esthor:lint/effect-subpath-imports

Conversation

@esthor

@esthor esthor commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Problem

docs/internals/effect-services.md says to import Effect modules as namespaces from their subpaths, and about 9,400 imports do, but nothing enforces it. Five files and one docs example still import from the bare effect package. The Effect language service doesn't flag them: main typechecks with all five.

Change

  • effect joins RESTRICTED_IMPORT_PATHS in vite.config.ts, next to the other restricted imports.
  • The five files and the example move to subpath namespace imports.
  • tsconfig.base.json drops "importFromBarrel": "error". That rule came from the TypeScript-plugin language service, and Effect TSGo never ported it, so the line has done nothing since the move to TSGo (Migrate TypeScript checks to Effect TSGo #2851). Newer TSGo releases fail on unknown rule names.

Scope and approval

Lint config plus import-only changes; no behavior changes. It enforces a rule the docs already state. I work at CodeRabbit.

Verification

  • A file importing from effect fails vp lint with the new message. No source file does; the remaining matches are string fixtures in tests and vendored patches/.
  • The contracts and server typechecks pass.
  • The contracts preview tests and the stack-retention memory test (which runs the fixture) pass: 25 tests.

Nearly every file imports Effect modules as namespaces from their
subpaths, but nothing enforced it: six files and one docs example still
imported from the bare `effect` package.

Restrict the bare `effect` import next to the other restricted imports,
and move those files and the example to subpath namespace imports.
@github-actions github-actions Bot added vouch:unvouched PR author is not yet trusted in the VOUCHED list. size:S 10-29 changed lines (additions + deletions). labels Oct 6, 2026
macroscopeapp[bot]
macroscopeapp Bot previously approved these changes Oct 6, 2026
@macroscopeapp

macroscopeapp Bot commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Approvability

Verdict: Approved at e0a780f

Macroscope's review found this PR approvable — This is a contained import-style migration plus lint enforcement; the source changes preserve existing Effect and Schema usage, while the configuration change only affects developer tooling. No product defaults, runtime workflows, schemas, or sensitive areas are modified.

You can add or adjust custom eligibility rules. Learn more.

@coderabbitai

coderabbitai Bot commented Oct 6, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Path: .coderabbit.config.ts
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 5d864c15-a50f-43d5-b08a-a5a8ef05cf10
📥 Commits

Reviewing files that changed from the base of the PR and between ed5ed5f and e04b045.

📒 Files selected for processing (1)
  • tsconfig.base.json
💤 Files with no reviewable changes (1)
  • tsconfig.base.json

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 8 remain after this review.


📝 Walkthrough

Walkthrough

Imports from the effect package root now use explicit subpaths across server code, contracts, tests, and documentation. The Vite configuration adds a restriction for package-root imports, and the TypeScript configuration removes the importFromBarrel diagnostic setting.

Changes

Effect import paths

Layer / File(s) Summary
Migrate imports to explicit subpaths
apps/server/src/git/testing/StackRetention.fixture.mjs, apps/server/src/preview/Manager.test.ts, packages/contracts/src/*, docs/operations/observability.md
Imports now use explicit effect/* subpaths. Schema definitions and test behavior remain unchanged.
Restrict package-root imports
vite.config.ts, tsconfig.base.json
The Vite restricted-import configuration adds a rule for imports from the effect package root. The TypeScript configuration no longer sets importFromBarrel to "error".

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Other

Merge Risk: ⚪ Minimal · up to e04b0

The import convention is enforced for the affected source files, with no identified issue preventing merge after normal checks.

Architecture Summary

Architecture risk: 🔵 Low · up to ed5ed

The change affects 4 systems.

Changed systems: vite.config.ts, packages/contracts, apps/server, docs

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

  • observed — vite.config.ts (service) was modified; 1 changed file maps to changed impact.
  • observed — packages/contracts (library) was modified; 4 changed files map to changed impact.
  • observed — apps/server (service) was modified; 2 changed files map to changed impact.
  • observed — docs (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

  • observed — Modified behavior in apps/server/src/git/testing/StackRetention.fixture.mjs: The fixture replaces the combined effect package import with separate imports from effect/Cache and effect/Effect.
  • observed — Modified behavior in apps/server/src/preview/Manager.test.ts: Replace named imports from effect with namespace imports from effect/Effect and effect/PubSub.
  • observed — Modified behavior in docs/operations/observability.md: The example switches from a named Effect import from effect to a namespace import from effect/Effect.
  • observed — Modified behavior in packages/contracts/src/device.ts: The schema import changes from the named Schema export of effect to the Schema namespace from effect/Schema.
🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 7…
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly describes the main change: importing Effect modules from subpaths. It is concise and uses a conventional commit format.
Description check ✅ Passed The description covers the problem, change, scope and approval rationale, and verification results. It also states the reported test and typecheck outcomes.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Comment @coderabbitai help to get the list of available commands.

@juliusmarminge

Copy link
Copy Markdown
Member

@coderabbitai why isn't the effect lsp catching this?? https://github.com/pingdotgg/t3code/blob/main/tsconfig.base.json#L26

@coderabbitai

coderabbitai Bot commented Oct 6, 2026

Copy link
Copy Markdown

Tip

For best results, initiate chat on the files or code changes.

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
fd '^tsconfig\.base\.json$'
cat -n tsconfig.base.json
rg -n -C3 '"@effect/language-service"|namespaceImport|effect/language-service|patch.*tsc|effect-language' --glob '!patches/**' --glob '!bun.lock' --glob '!pnpm-lock.yaml' .
fd '^(package\.json|tsconfig.*\.json|.*lock.*)$' -d 3
ast-grep outline vite.config.ts --match 'RESTRICTED_IMPORT_PATHS' --view expanded

Length of output: 26803


🏁 Script executed:

#!/bin/bash
cat -n package.json
cat -n packages/contracts/package.json
cat -n packages/contracts/tsconfig.json
cat -n apps/server/package.json
cat -n apps/server/tsconfig.json
rg -n -C2 'language-service|tsgo|typescript|effect.*patch' pnpm-lock.yaml
fd -H '^(settings\.json|extensions\.json)$' --exclude .git
rg -n -C2 'effect/language-service|effect-language-service|tsdk|typescript-go|tsgo|prepare|postinstall' --glob '*.json' --glob '*.yaml' --glob '*.yml' --glob '!**/fixtures/**' --hidden --glob '!.git/**' .

Length of output: 153901


🤖 web_search executed:

Effect TypeScript language service importFromBarrel diagnostic namespaceImportPackages compiler tsc tsgo patch @effect/language-service documentation

Length of output: 1620


🏁 Script executed:

#!/bin/bash
cat -n .vscode/settings.json
cat -n .vscode/extensions.json
gh api repos/Effect-TS/tsgo/git/trees/v0.41.0?recursive=1 --jq '.tree[] | select(.path | test("import.*barrel|import.*namespace|README|diagnostic|config"; "i")) | .path' | head -100
curl -fsSL https://raw.githubusercontent.com/Effect-TS/tsgo/v0.41.0/README.md | sed -n '1,240p'

Length of output: 1382


🤖 web_search executed:

site:github.com/Effect-TS/tsgo "importFromBarrel" "0.41" OR namespaceImportPackages OR diagnostic

Length of output: 393


🏁 Script executed:

#!/bin/bash
curl -fsSL https://registry.npmjs.org/@effect%2ftsgo/0.41.0 | jq '{version,gitHead,repository,dist}'
gh api repos/Effect-TS/tsgo --jq '{default_branch,html_url}'
gh api 'repos/Effect-TS/tsgo/git/trees/main?recursive=1' --jq '.tree[] | select(.path | test("barrel|namespace|README|diagnostic|config"; "i")) | .path' | head -100
curl -fsSL https://raw.githubusercontent.com/Effect-TS/tsgo/main/README.md | head -220

Length of output: 41904


🏁 Script executed:

#!/bin/bash
gh api 'repos/Effect-TS/tsgo/tags?per_page=100' --jq '.[] | select(.name | contains("0.41.0")) | {name,commit}'
gh api 'repos/Effect-TS/tsgo/git/trees/main?recursive=1' --jq '.tree[] | select(.path | test("effectoptions|options.go|rules.go|rule.*registry|import"; "i")) | .path' | head -90
curl -fsSL https://raw.githubusercontent.com/Effect-TS/tsgo/main/internal/effectconfigcheck/validation.go | grep -n -C4 -E 'importFromBarrel|namespaceImport|unknown|Unknown'
curl -fsSL https://raw.githubusercontent.com/Effect-TS/tsgo/main/README.md | grep -n -C3 -E 'importFromBarrel|namespaceImportPackages|barrelImportPackages|not.*port|unsupported'

Length of output: 9616


🏁 Script executed:

#!/bin/bash
set -euo pipefail
ref=71ced39cc84282848bde2098718f63ca9755831b
curl -fsSL "https://raw.githubusercontent.com/Effect-TS/tsgo/$ref/internal/rules/rules.go" | sed -n '1,210p'
curl -fsSL "https://raw.githubusercontent.com/Effect-TS/tsgo/$ref/README.md" | grep -n -C3 -E 'importFromBarrel|namespaceImportPackages|barrelImportPackages|auto.import|import style'
gh api "repos/Effect-TS/tsgo/git/trees/$ref?recursive=1" --jq '.tree[] | select(.path | test("barrel|autoimportstyle|options.go"; "i")) | .path'
curl -fsSL "https://raw.githubusercontent.com/Effect-TS/tsgo/$ref/internal/autoimportstyle/stylepolicy.go" | head -100

Length of output: 7687


@juliusmarminge importFromBarrel is not implemented in the native @effect/tsgo version this repo uses.

The repo uses TypeScript 7.0.2 with @effect/tsgo 0.41.0. Its prepare script runs effect-tsgo patch, so this is not the usual “language-service plugins only run in the editor” issue.

I checked the exact 0.41.0 release:

So "importFromBarrel": "error" at tsconfig.base.json Line 26 configures a legacy language-service rule that this native release cannot run. The new restricted-import lint rule closes that enforcement gap.

You are interacting with an AI system.

`importFromBarrel` came from the TypeScript-plugin language service.
Effect TSGo never ported it, and 0.41 accepts unknown rule names without
a warning, so the line has done nothing since the move to TSGo. The
restricted `effect` import enforces it now, and newer TSGo releases
fail on unknown rule names.
@esthor

esthor commented Oct 6, 2026

Copy link
Copy Markdown
Contributor Author

To add the "why silently" part: tsgo 0.41 accepts unknown diagnosticSeverity keys without a warning (Effect-TS/tsgo#747, fixed in 0.46.0), so the line has done nothing since the move to TSGo in #2851. I dropped it in e04b045. An @effect/tsgo 0.46.1 upgrade follows in its own PR.

@github-actions github-actions Bot added vouch:trusted PR author is trusted by repo permissions or the VOUCHED list. and removed vouch:unvouched PR author is not yet trusted in the VOUCHED list. labels Oct 6, 2026
…mports

# Conflicts:
#	apps/server/src/preview/Manager.test.ts
@macroscopeapp
macroscopeapp Bot dismissed their stale review October 6, 2026 07:10

Dismissing prior approval to re-evaluate e0a780f

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size:S 10-29 changed lines (additions + deletions). vouch:trusted PR author is trusted by repo permissions or the VOUCHED list.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants