Conversation
🦋 Changeset detectedLatest commit: 2e793af The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis update introduces path alias support for GraphQL import statements, modeled after TypeScript's Changes
Sequence Diagram(s)sequenceDiagram
participant Loader as GraphQLFileLoader
participant Import as processImport
participant Resolver as resolveFilePath
participant FS as File System
Loader->>Import: processImport(pointer, cwd, ..., pathAliases)
Import->>Resolver: resolveFilePath(importPath, pathAliases)
alt Alias matches
Resolver->>FS: Resolve mapped path (using alias)
else No alias match
Resolver->>FS: Resolve original path
end
FS-->>Resolver: Return resolved path
Resolver-->>Import: Resolved file path
Import-->>Loader: Imported schema
Estimated code review effort4 (~90 minutes) Possibly related issues
Suggested reviewers
Poem
✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (6)
packages/import/tests/schema/fixtures/path-aliases/mixed-use/schema.graphql (1)
1-3: Standardise the# importdirective and double-check the last path
The first two statements omit the space after
#, the third keeps the same style, yet in the rest of the fixture set you use# import …(with a space). Pick one convention (the spec & graphql-tools docs show# import …) and use it everywhere.
"c-graphql"lacks an extension and is the only import that relies on an implicit resolver. Unless you intentionally test that branch, suffix it with.graphqlto avoid false negatives when another import resolver is used.-#import "@mixed-use/a.graphql" -#import "./b.graphql" -#import "c-graphql" +# import "@mixed-use/a.graphql" +# import "./b.graphql" +# import "c-graphql.graphql"Ensure the alias map in the test config resolves
c-graphql(.graphql)exactly once; otherwise the test may silently pass with an unintended file.packages/import/tests/schema/fixtures/path-aliases/circular/b.graphql (1)
1-1: Minor: keep directive style consistentOther fixtures use
# import, here you already follow that style – good. Just make sure the surrounding files in the same folder do the same to keep grep/regex rules simple.packages/import/tests/schema/fixtures/path-aliases/multiple-levels/level1.graphql (1)
1-1: Consistent quoting & directive spacingThis fixture adopts the
# import+ single quotes convention, which is preferred. Consider normalising the mixed-use fixture to match (see above) so the test corpus is homogeneous.packages/import/tests/schema/fixtures/path-aliases/multiple-levels/level2.graphql (1)
1-1: Nit: drop the leading@in the comment to align with other fixturesNot blocking, but the other multi-level files omit the
@beforeimportin the comment itself (# import …). Uniform comments make pattern matching for fixtures easier.packages/import/src/index.ts (2)
669-694: Path resolution logic correctly prioritizes aliases.The implementation properly attempts alias resolution first before falling back to the original logic, maintaining backward compatibility.
Consider adding error handling for edge cases:
function resolveFilePath(filePath: string, importFrom: string, pathAliases?: PathAliases): string { // First, check if importFrom matches any path aliases. if (pathAliases != null) { for (const [prefixPattern, mapping] of Object.entries(pathAliases.mappings)) { const matchedMapping = applyPathAlias(prefixPattern, mapping, importFrom); if (matchedMapping == null) { continue; } const resolvedMapping = resolveFrom(pathAliases.rootDir ?? process.cwd(), matchedMapping); - return realpathSync(resolvedMapping); + try { + return realpathSync(resolvedMapping); + } catch (e: any) { + // If alias resolution fails, continue to next alias or fall back + if (e.code !== 'ENOENT') { + throw e; + } + } } }
672-673: Consider validating path alias mappings.While the implementation is correct, consider adding validation to ensure mapping patterns are well-formed to provide better error messages to users.
if (pathAliases != null) { + // Validate mappings + for (const [pattern, mapping] of Object.entries(pathAliases.mappings)) { + if (!pattern || typeof mapping !== 'string') { + throw new Error(`Invalid path alias mapping: "${pattern}" -> "${mapping}"`); + } + } for (const [prefixPattern, mapping] of Object.entries(pathAliases.mappings)) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
packages/import/src/index.ts(8 hunks)packages/import/tests/schema/fixtures/path-aliases/circular/a.graphql(1 hunks)packages/import/tests/schema/fixtures/path-aliases/circular/b.graphql(1 hunks)packages/import/tests/schema/fixtures/path-aliases/exact/a.graphql(1 hunks)packages/import/tests/schema/fixtures/path-aliases/exact/b.graphql(1 hunks)packages/import/tests/schema/fixtures/path-aliases/mixed-use/a.graphql(1 hunks)packages/import/tests/schema/fixtures/path-aliases/mixed-use/b.graphql(1 hunks)packages/import/tests/schema/fixtures/path-aliases/mixed-use/c.graphql(1 hunks)packages/import/tests/schema/fixtures/path-aliases/mixed-use/schema.graphql(1 hunks)packages/import/tests/schema/fixtures/path-aliases/multiple-levels/level1.graphql(1 hunks)packages/import/tests/schema/fixtures/path-aliases/multiple-levels/level2.graphql(1 hunks)packages/import/tests/schema/fixtures/path-aliases/multiple-levels/level3.graphql(1 hunks)packages/import/tests/schema/fixtures/path-aliases/multiple-levels/level4.graphql(1 hunks)packages/import/tests/schema/import-schema.spec.ts(2 hunks)packages/loaders/graphql-file/src/index.ts(3 hunks)
🧠 Learnings (2)
📓 Common learnings
Learnt from: ardatan
PR: ardatan/graphql-tools#0
File: :0-0
Timestamp: 2025-01-29T19:58:05.749Z
Learning: The `isUrl` helper in @graphql-tools/utils should be tested with both URL.canParse and new URL() paths, covering various URL formats (http, https, file) and invalid cases. Tests should properly mock and restore URL.canParse to ensure consistent behavior across environments.
packages/import/tests/schema/import-schema.spec.ts (1)
Learnt from: ardatan
PR: ardatan/graphql-tools#0
File: :0-0
Timestamp: 2025-01-29T19:58:05.749Z
Learning: The isUrl helper in @graphql-tools/utils should be tested with both URL.canParse and new URL() paths, covering various URL formats (http, https, file) and invalid cases. Tests should properly mock and restore URL.canParse to ensure consistent behavior across environments.
🧬 Code Graph Analysis (1)
packages/loaders/graphql-file/src/index.ts (1)
packages/import/src/index.ts (2)
PathAliases(68-127)processImport(134-166)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: ardatan
PR: ardatan/graphql-tools#0
File: :0-0
Timestamp: 2025-01-29T19:58:05.749Z
Learning: The `isUrl` helper in @graphql-tools/utils should be tested with both URL.canParse and new URL() paths, covering various URL formats (http, https, file) and invalid cases. Tests should properly mock and restore URL.canParse to ensure consistent behavior across environments.
packages/import/tests/schema/import-schema.spec.ts (1)
Learnt from: ardatan
PR: ardatan/graphql-tools#0
File: :0-0
Timestamp: 2025-01-29T19:58:05.749Z
Learning: The isUrl helper in @graphql-tools/utils should be tested with both URL.canParse and new URL() paths, covering various URL formats (http, https, file) and invalid cases. Tests should properly mock and restore URL.canParse to ensure consistent behavior across environments.
🧬 Code Graph Analysis (1)
packages/loaders/graphql-file/src/index.ts (1)
packages/import/src/index.ts (2)
PathAliases(68-127)processImport(134-166)
🔇 Additional comments (16)
packages/import/tests/schema/fixtures/path-aliases/mixed-use/c.graphql (1)
1-3: Fixture is concise and valid.GraphQL syntax and minimal field set are perfect for the alias-resolution test case.
packages/import/tests/schema/fixtures/path-aliases/exact/b.graphql (1)
1-3: LGTM.
TypeBis well-formed and adequate for the exact-alias fixture.packages/import/tests/schema/fixtures/path-aliases/mixed-use/a.graphql (1)
1-3: No issues detected.Schema is syntactically correct and serves its purpose in the mixed-alias scenario.
packages/import/tests/schema/fixtures/path-aliases/mixed-use/b.graphql (1)
1-3: Looks good.
Btype definition is valid and appropriate for the test fixture.packages/import/tests/schema/fixtures/path-aliases/exact/a.graphql (2)
1-1: Confirm loader accepts# importsyntax.The import directive uses
# import(with a space). Ensure the updated loader continues supporting both#importand# importforms so this fixture is parsed.
3-10: Schema content is valid.Types and fields are well-structured for the exact-alias test case.
packages/import/tests/schema/fixtures/path-aliases/multiple-levels/level4.graphql (1)
1-7: Fixture looks goodPure type definitions, schema is syntactically valid and fulfils its role in the deep-alias chain.
packages/import/tests/schema/fixtures/path-aliases/circular/a.graphql (1)
1-11: Test fixture correctly sets up circular dependency scenario.The schema properly defines the circular reference test case with TypeA referencing TypeB through the
relatedBfield, and the import statement uses the path alias syntax as expected.packages/import/tests/schema/fixtures/path-aliases/multiple-levels/level3.graphql (1)
1-11: Multi-level import test fixture is well-structured.The schema correctly demonstrates multi-level path alias imports by referencing
Productstype from level4.graphql through the aliased import path.packages/loaders/graphql-file/src/index.ts (3)
7-7: Import statement correctly includes PathAliases type.The addition of
PathAliasesto the import statement properly supports the new feature.
38-41: Interface extension properly documents path aliases option.The
pathAliasesproperty is well-documented and correctly typed as optional.
218-218: processImport call correctly passes path aliases parameter.The function call properly includes the new parameters with appropriate default values for
predefinedImportsandvisitedFiles.packages/import/tests/schema/import-schema.spec.ts (2)
2-13: Helper function correctly updated to support path aliases.The
importSchemafunction properly accepts and passes thepathAliasesparameter toprocessImport.
1154-1324: Comprehensive test coverage for path aliasing feature.The test suite excellently covers various path aliasing scenarios including:
- Exact path aliases
- Wildcard patterns
- Mixed usage with relative imports
- Multi-level nested aliases
- Circular dependencies
- Root directory resolution
All test cases properly use
path.joinfor cross-platform compatibility.packages/import/src/index.ts (2)
64-127: Well-designed PathAliases interface with excellent documentation.The interface follows TypeScript conventions and provides comprehensive documentation with clear examples. The support for both exact and wildcard mappings aligns perfectly with the PR objectives.
696-732: applyPathAlias correctly implements TypeScript path mapping semantics.The function properly handles all the documented mapping patterns including exact matches and wildcards with proper remainder handling.
|
Thanks for the PR! |
done! thanks for the review :) |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
.changeset/tired-foxes-join.md (1)
30-30: Use the compound adjective “backward-compatible”Minor language nit:
-This change is introduced in a backwards compatible way +This change is introduced in a backward-compatible way
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.changeset/tired-foxes-join.md(1 hunks)
🧠 Learnings (1)
📓 Common learnings
Learnt from: ardatan
PR: ardatan/graphql-tools#0
File: :0-0
Timestamp: 2025-01-29T19:58:05.749Z
Learning: The `isUrl` helper in @graphql-tools/utils should be tested with both URL.canParse and new URL() paths, covering various URL formats (http, https, file) and invalid cases. Tests should properly mock and restore URL.canParse to ensure consistent behavior across environments.
🪛 LanguageTool
.changeset/tired-foxes-join.md
[uncategorized] ~30-~30: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...} } ``` This change is introduced in a backwards compatible way to ensure no existing use cases are...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 markdownlint-cli2 (0.17.2)
.changeset/tired-foxes-join.md
20-20: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: ardatan
PR: ardatan/graphql-tools#0
File: :0-0
Timestamp: 2025-01-29T19:58:05.749Z
Learning: The `isUrl` helper in @graphql-tools/utils should be tested with both URL.canParse and new URL() paths, covering various URL formats (http, https, file) and invalid cases. Tests should properly mock and restore URL.canParse to ensure consistent behavior across environments.
🪛 LanguageTool
.changeset/tired-foxes-join.md
[uncategorized] ~30-~30: If this is a compound adjective that modifies the following noun, use a hyphen.
Context: ...} } ``` This change is introduced in a backwards compatible way to ensure no existing use cases are...
(EN_COMPOUND_ADJECTIVE_INTERNAL)
🪛 markdownlint-cli2 (0.17.2)
.changeset/tired-foxes-join.md
20-20: Emphasis used instead of a heading
(MD036, no-emphasis-as-heading)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Unit Test on Node 18 (ubuntu-latest) and GraphQL v15
- GitHub Check: Unit Test on Node 18 (ubuntu-latest) and GraphQL v16
- GitHub Check: Unit Test on Node 22 (ubuntu-latest) and GraphQL v16
- GitHub Check: Unit Test on Node 18 (windows-latest) and GraphQL v16
- GitHub Check: Full Check on GraphQL v16
- GitHub Check: Unit Test on Node 24 (ubuntu-latest) and GraphQL v16
- GitHub Check: Unit Test on Bun
🔇 Additional comments (1)
.changeset/tired-foxes-join.md (1)
20-27: Align config snippet with the actual option name exposed by the codeIn the implementation, the option was added under
pathAliases(seeGraphQLFileLoaderOptions.pathAliases).
The snippet here showsmappings, which could mislead users and break copy-pasted examples.Please double-check the exported option name and keep the doc consistent with the code.
|
Merged 👍 |
* import aliases * self review * fix typo * address comments * changeset
2 participants
Description
GraphQL schemas in large projects, especially monorepos, suffer from fragile and verbose relative import paths that become difficult to maintain as projects grow. This PR brings TypeScript's popular
tsconfig.json#pathsaliasing syntax to GraphQL imports, enabling clean, maintainable import statements across your GraphQL schema files.Before - Brittle relative imports:
After - Clean, semantic aliases:
Configuration Example
This change is introduced in a backwards compatible way to ensure no existing use cases are broken while using familiar patterns to typescript developers for structuring import aliases.
Related #7311
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Test Environment:
@graphql-tools/...: importChecklist:
CONTRIBUTING doc and the
style guidelines of this project