Skip to content

Load config from apollo.config.js or package.json#497

Merged
shadaj merged 24 commits intomasterfrom
apollo-config-loading
Jul 20, 2018
Merged

Load config from apollo.config.js or package.json#497
shadaj merged 24 commits intomasterfrom
apollo-config-loading

Conversation

@shadaj
Copy link
Copy Markdown
Contributor

@shadaj shadaj commented Jul 17, 2018

Needs documentation, probably in a separate PR, but the implementation is ready.

@shadaj shadaj self-assigned this Jul 17, 2018
@ghost ghost added the 🎉 feature New addition or enhancement to existing solutions label Jul 17, 2018
Copy link
Copy Markdown
Contributor

@jbaxleyiii jbaxleyiii left a comment

Choose a reason for hiding this comment

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

@shadaj looks like a lot of formatting noise here?

I've left a couple comments on the loadingConfig function(s).

@martijnwalraven will have some feedback on the config format as well. Lets block on that

if (flags.config) {
ctx.config = loadConfigFromFile(flags.config) || {};
} else {
ctx.config = findAndLoadConfig(resolve(".")) || {};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

what is the goal of resolve(".") here?

await promisify(fs.writeFile)(args.output, JSON.stringify(ctx.schema));
},
if (flags.config) {
ctx.config = loadConfigFromFile(flags.config) || {};
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it feels like we should be able to combine this and the next function for a shared loadConfig(path?) option that defaults to process.cwd()

? [obj.operations]
: obj.operations
? (obj.operations as string[])
: ["**/*.graphql"],
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

lets make sure to ignore node_modules by default

@shadaj shadaj force-pushed the apollo-config-loading branch from 7610d4f to 55b5459 Compare July 19, 2018 19:12
Copy link
Copy Markdown
Contributor

@jbaxleyiii jbaxleyiii left a comment

Choose a reason for hiding this comment

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

@shadaj what does the load order look like if mutliple schema values are configured?

I.e. if I include a url + engineKey? Ideally we would want to check the url and if not found load from engine?

}
task.title = `Scanning for GraphQL queries (${(ctx.documentSets as ResolvedDocumentSet[])
.map(s => s.documentPaths.length)
.reduce((a, b) => a + b, 0)} found)`; // this is a bogus wrong value
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is this bogus because its files not operations?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Oops! It used to be really bogus (counted the number of document sets) before. I'll drop the comment.

Comment thread packages/apollo-cli/src/config.ts Outdated

export interface ApolloConfig {
projectFolder: string;
projectName?: string;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

can we change this to be just name?

What is the goal of projectFolder?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

projectFolder tracks where the root of the project is for handling relative glob paths. It can't be picked up from cwd always (in VSCode for example), so we treat it as a config value.

schema?: string;
endpoint?: EndpointConfig;
engineKey?: string;
extends?: string;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

how do we handle multiple extends? I'm assuming right now we don't

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

So for me multiple extends didn't make too much sense because you're taking a schema and adjusting it. I'm not sure what it would mean to extend multiple schemas. In the future though, I would like to support multiple documents extending a schema in the case that you split up your types in multiple files.

@shadaj
Copy link
Copy Markdown
Contributor Author

shadaj commented Jul 20, 2018

The order for loading schemas is based on which data is closest:

  1. The schema value (if it's a file read it, if it's a URL introspect it)
  2. The endpoint value (introspect it)
  3. The engine key (grab the latest uploaded schema)

@ghost ghost added the 🎉 feature New addition or enhancement to existing solutions label Jul 20, 2018
@shadaj shadaj merged commit decc9ed into master Jul 20, 2018
@shadaj shadaj deleted the apollo-config-loading branch July 20, 2018 21:32
@homburg homburg mentioned this pull request Dec 7, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🎉 feature New addition or enhancement to existing solutions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants