Adding ability to process js, graphql, and other arbitrary file types (e.g. .gql) #51
Conversation
… (e.g. gql files)
|
@mergebandit: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/ |
| @@ -61,7 +61,7 @@ export class ExtractGQL { | |||
| public queryTransformers: QueryTransformer[] = []; | |||
|
|
|||
| // The file extension to load queries from | |||
| public extension: string; | |||
| public extension: Array<string> = ['graphql']; | |||
There was a problem hiding this comment.
We should rename this to be a plural thing rather than leave it "extension" since it is now an array
There was a problem hiding this comment.
The only reason I didn't change this is the CLI arg is extension - and since renaming the arg would be a breaking change, elected to go with consistency.
Obv easy / happy to do this, but wanted to explain my thinking.
| inJsCode = false, | ||
| }: ExtractGQLOptions) { | ||
| this.inputFilePath = inputFilePath; | ||
| this.outputFilePath = outputFilePath; | ||
| this.queryTransformers = queryTransformers; | ||
| this.extension = extension; | ||
| this.extension = this.extension.concat(extension.split(',').map(e => e.trim())) |
There was a problem hiding this comment.
Instead of doing this, we should just have a this.extensions that we can pass in.
There was a problem hiding this comment.
Can you clarify this? If it's coming from the CLI how else do you propose I pass in an array of things? I could just let the variable be a string, and split it later?
| @@ -193,8 +193,8 @@ export class ExtractGQL { | |||
| return Promise.resolve().then(() => { | |||
| const extension = ExtractGQL.getFileExtension(inputFile); | |||
|
|
|||
| if (extension === this.extension) { | |||
| if (this.inJsCode) { | |||
| if (this.extension.indexOf(extension) > -1) { | |||
There was a problem hiding this comment.
Similarly, should just be able to an array "contains" check rather than doing indexing on a string
There was a problem hiding this comment.
I assume you mean array.includes(val) - the reason I didn't do this is the tsconfig.json is set for es6, and array.includes(val) is part of the es7 lib. Do you want me to include that?
|
@mergebandit Thanks so much for the PR! I mentioned a couple of changes above that should be pretty easy to implement. I think the concept is fine otherwise. |
|
@Poincare - left some responses above. |
|
@mergebandit Yup saw those - will respond to each in about 1-2 days. |
|
Bump. |
|
Also - I'll need to bump the minor version in my next commit (reminder to myself) |
|
@Poincare I (and others on my team) would be more than happy to assist in ongoing maintenance and feature development for this plugin. |
|
Is this ever going to be addressed or should I just maintain a fork moving forward? |
extensionproperty / argument now supports multiple file types.This allows users to execute
persistgraphqlin environments that utilize thegqltagged template literal,.graphqlfiles,.gqlfiles, and any arbitrary extension that consumers have defined for graphql files.Specifically, the
persistgraphql-webpack-plugindefines a loader for.graphql|.gqlfiles, which was my original aim to support.Fixes #50