Skip to content

Conversation

@mattietea
Copy link

@mattietea mattietea commented Oct 21, 2025

Description

It seems loadDocument and loadDocumentSync reference don't account for the project's exclude property resulting in unnecessary files getting loaded into memory.

Fixes #1748

Type of change

  • Breaking change (fix or feature that would cause existing functionality to not work as expected)

Screenshots/Sandbox (if appropriate/relevant):

Reproduction PR

How Has This Been Tested?

Added tests to the PR

Test Environment:

  • OS: MacOS 26.0.1
  • GraphQL Config Version: 5.1.5
  • NodeJS: v24.9.0

Checklist:

  • I have followed the CONTRIBUTING doc and the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests and linter rules pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Further comments

If this is a relatively large or complex change, kick off the discussion by explaining why you chose the solution you did and what alternatives you considered, etc...

  • I kept support for ignore, as I'm not sure how widely used ignore is. Another alternative could be to rename ignore to exclude?
  • I noticed this error when using the eslint-graphql plugin. let me know if this isn't the right place to fix this!

@mattietea mattietea force-pushed the mattietea_use-exclude-when-loading-documents branch from ffa7ed7 to d78f09e Compare October 21, 2025 14:08
return this._extensionsRegistry.loaders.documents.loadDocuments(pointer, options);
return this._extensionsRegistry.loaders.documents.loadDocuments(pointer, {
...options,
ignore: options?.ignore ? options?.ignore : this.exclude,
Copy link
Author

Choose a reason for hiding this comment

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

Opted to make ignore override exclude incase people are already using ignore. A couple other options,

  • Merge ignore and exclude
  • Rename ignore to exclude
    • If exclude is passed as an option, should that be merged with this.exclude?

@mattietea mattietea marked this pull request as ready for review October 24, 2025 16:50
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.

loadDocument and loadDocumentSync loading excluded files

1 participant