feat(git-loader): add glob support #3012
Conversation
|
| @@ -58,6 +67,11 @@ export class CodeFileLoader implements UniversalLoader<CodeFileLoaderOptions> { | |||
| pointer: SchemaPointerSingle | DocumentPointerSingle, | |||
| options: CodeFileLoaderOptions | |||
| ): Promise<boolean> { | |||
| if (isGlob(pointer)) { | |||
| // FIXME: parse to find and check the file extensions? | |||
There was a problem hiding this comment.
Not sure how though. Maybe picomatch.parse? 🤔
There was a problem hiding this comment.
Oh, maybe this is a non-issue: the graphql-file and code-file loaders can both resolve globs for the other because they're both on the file system? So as long as there isn't a protocol prefix like git: in front (or is a file: protocol though that might be a separate feature PR) blindly returning true for all (local file system) globs is fine?
There was a problem hiding this comment.
@PixnBits It is kind of tricky but let's try that and see if the tests would pass or fail
| @@ -31,6 +36,10 @@ function isGraphQLImportFile(rawSDL: string) { | |||
| return trimmedRawSDL.startsWith('# import') || trimmedRawSDL.startsWith('#import'); | |||
| } | |||
|
|
|||
| function createGlobbyOptions(options: GraphQLFileLoaderOptions): GlobbyOptions { | |||
There was a problem hiding this comment.
used by both code file and graphql file loaders, might be a candidate for moving to the utils package? or the filesystem glob calls rather than something specific to globby
not sure and not advocating for the move 😅
|
@PixnBits Thank you very much! This PR looks awesome! Do you this is ready to merge? Is there anything we can help you? |
|
Very very sorry to respond late, I was curious if this was the right/an acceptable direction and was going to start on tests too, but got a bit busy 😓 |
|
@PixnBits Thank you :) |
Description
While Providing Schema GraphQL file doesn't mention globs, Providing Documents GraphQL file does. It could also be useful for the git repository notation to likewise support globs. An example for graphql-inspector:
This PR is the start of a possible implementation. It's my first TypeScript project so my intentions may not be expressed in the canonical way, I'm moving glob resolution out of the source collector to the individual loaders, and other bull-in-the-china-shop moves. 😅 I can start on tests, but figured it might be better to see if the approach needed redirection first.
Related #2786
Type of change
Please delete options that are not relevant.
Possibly a breaking change to the interface of GraphQLFileLoader, uncertain so retaining the option for now but can delete that if seen as not a breaking change.
How Has This Been Tested?
I've been using these changes on a project, using just the file and git loaders. I
$ yarn packd in the relevant package directories and installed the resulting tgz's in the other project and used the git glob syntax successfully.Checklist: