Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ENHANCEMENT: Add nested fields, args, distribute args to fields #683

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion client/dist/js/bundle.js

Large diffs are not rendered by default.

100 changes: 79 additions & 21 deletions client/src/lib/dependency-injection/ApolloGraphqlManager.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
import { graphql } from 'react-apollo';
import gql from 'graphql-tag';
import { ROOT_FIELD } from './graphql/helpers';

const TEMPLATE_OVERRIDE = '__TEMPLATE_OVERRIDE__';
const protectedConfig = ['templateName', 'fields', 'params', 'fragments'];
const deferredApolloConfig = ['options', 'props', 'variables', 'skip', 'update'];
const configDefaults = {
params: [],
params: {},
args: {},
fields: [],
fragments: [],
pagination: true,
Expand Down Expand Up @@ -75,6 +77,10 @@ class ApolloGraphqlManager {
...configDefaults,
...config,
};
// Fields array is mutated by reference, so this has to be dereferenced from the configDefaults
mergedConfig.fields = [
...mergedConfig.fields
];
const {
apolloConfig,
...otherConfig
Expand Down Expand Up @@ -130,55 +136,107 @@ class ApolloGraphqlManager {

/**
* Adds a param to the query
* @param param
*
* @param {string} name
* @param {string} type
* @returns {ApolloGraphqlManager}
*/
addParam(param) {
return this.addParams([param]);
addParam(name, type) {
if (!name || !type) {
throw new Error('addParam must be passed a name and type parameter');
}
return this.addParams({
[name]: type
});
}

/**
* Adds multiple params to the query
* @param params
*
* @param {object} params
* @returns {ApolloGraphqlManager}
*/
addParams(params = []) {
addParams(params = {}) {
const existing = this.config.params;
this.config.params = [
this.config.params = {
...existing,
...params,
];
};

return this;
}

/**
* Adds an arg to the query
*
* @param {string} path The path to the field where the args are applied
* @param {string} name
* @param {string} variableName
* @returns {ApolloGraphqlManager}
*/
addArg(path = ROOT_FIELD, name, variableName) {
return this.addArgs(path, {
[name]: variableName
});
}

/**
* Adds multiple args to the query
*
* @param {string} path The path to the field where the args are applied
* @param {object} args
* @returns {ApolloGraphqlManager}
*/
addArgs(path = ROOT_FIELD, args = {}) {
const existing = this.config.args[path] || {};
this.config.args[path] = {
...existing,
...args,
};

return this;
}

/**
* Adds a field to the query
* @param field
*
* @param {string} path
* @param {string} field
* @returns {ApolloGraphqlManager}}
*/
addField(field) {
return this.addFields([field]);
addField(path = ROOT_FIELD, field) {
robbieaverill marked this conversation as resolved.
Show resolved Hide resolved
return this.addFields(path, [field]);
}

/**
* Adds a list of fields to the query
* @param fields
*
* @param {string[]} fields
* @returns {ApolloGraphqlManager}
*/
addFields(fields = []) {
const existing = this.config.fields;
this.config.fields = [
...existing,
...fields,
];
addFields(path = ROOT_FIELD, fields = []) {
let fieldArray = [];
path.split('/').forEach(part => {
Copy link
Member

Choose a reason for hiding this comment

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

Is that an invalid symbol in GraphQL field names? I mean, it'd be a pretty weird field name, but who knows :D

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, deliberately so, because the / is a DSL for nested field names. Had to choose a character that was invalid.

if (part === ROOT_FIELD) {
fieldArray = this.config.fields;
} else {
const index = fieldArray.indexOf(part);
const next = fieldArray[index + 1];
if (index === -1 || !Array.isArray(next)) {
throw new Error(`Invalid path to field: ${path}`);
}

fieldArray = next;
}
});
fields.forEach(f => fieldArray.push(f));
return this;
}

/**
* Includes a fragment in the query
* @param name
*
* @param {string} name
* @returns {ApolloGraphqlManager}
*/
useFragment(name) {
Expand All @@ -193,8 +251,8 @@ class ApolloGraphqlManager {
/**
* Change to another template to use
*
* @param name
* @return {ApolloGraphqlManager}
* @param {string} name
* @returns {ApolloGraphqlManager}
*/
useTemplate(name) {
if (!Object.keys(this.templates).includes(name)) {
Expand Down
4 changes: 2 additions & 2 deletions client/src/lib/dependency-injection/graphql/buildReadQuery.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { defaultTag } from './tags';
import { getPluralName, getVariables, getParams, getFields, getFragments } from './helpers';
import { getPluralName, getVariables, getRootParams, getFields, getFragments } from './helpers';

const buildReadQuery = (tag = defaultTag) => (
tag`query Read${getPluralName}${getVariables} {
read${getPluralName}${getParams} {
read${getPluralName}${getRootParams} {
${getFields}
}
}
Expand Down
41 changes: 30 additions & 11 deletions client/src/lib/dependency-injection/graphql/helpers.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
export const ROOT_FIELD = 'root';

const paginationFields = {
limit: 'Int',
offset: 'Int',
Expand All @@ -24,22 +26,39 @@ export const getVariables = ({ params, pagination = true }) => {

export const getParams = ({ params, pagination = true }) => {
const items = (pagination) ? { ...params, ...paginationFields } : params;

const paramList = Object.keys(items)
.map((key) => (
`${key}: $${key}`
const paramList = Object.entries(items)
.map(([paramName, varName]) => (
`${paramName}: $${varName}`
));

return paramList.length ? `(${paramList.join(', ')})` : '';
};

export const getFields = ({ fields, pagination = true }) => {
const strings = fields.map(field => (
(Array.isArray(field))
// nested fields shouldn't also have pagination
? `{ ${getFields({ fields: field, pagination: false })} }`
: field
));
export const getRootParams = ({ args, pagination = true }) => {
const fieldParams = args[ROOT_FIELD] || {};

return getParams({ params: fieldParams, pagination });
};

export const getFields = ({ args, fields, pagination = true }, stack = [ROOT_FIELD]) => {
const strings = fields.map((field, i) => {
if (Array.isArray(field)) {
return `
{
${getFields(
{ args, fields: field, pagination: false },
[...stack, fields[i - 1]],
)}
}`;
}
const path = [...stack, field];
const key = path.join('/');
const fieldParams = args[key] || {};

const str = `${field}${getParams({ params: fieldParams, pagination: false })}`;

return str;
});
robbieaverill marked this conversation as resolved.
Show resolved Hide resolved

if (pagination) {
return paginateFields(strings);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/* global jest, jasmine, describe, beforeEach, it, pit, expect, process */

import ApolloGraphqlManager from '../ApolloGraphqlManager';
import { getFields } from '../graphql/helpers';

const createMock = (config = {}, templates = {}, fragments = {}) => (
new ApolloGraphqlManager(config, templates, fragments)
Expand Down Expand Up @@ -51,20 +52,23 @@ describe('ApolloGraphqlManager', () => {

it('adds params', () => {
const manager = createMock();
manager.addParam('test');
manager.addParams(['rest', 'fest']);
manager.addParam('test', 'String');
manager.addParams({
foo: 'Boolean',
baz: 'Int!'
});

const params = manager.getConfig().params;
expect(Array.isArray(params)).toBe(true);
expect(params).toContain('test');
expect(params).toContain('rest');
expect(params).toContain('fest');
expect(typeof params).toBe('object');
expect(params.test).toBe('String');
expect(params.foo).toBe('Boolean');
expect(params.baz).toBe('Int!');
});

it('adds fields', () => {
const manager = createMock();
manager.addField('test');
manager.addFields(['rest', 'fest']);
manager.addField('root', 'test');
manager.addFields('root', ['rest', 'fest']);

const fields = manager.getConfig().fields;
expect(Array.isArray(fields)).toBe(true);
Expand All @@ -73,6 +77,44 @@ describe('ApolloGraphqlManager', () => {
expect(fields).toContain('fest');
});

it('adds nested fields', () => {
const manager = createMock();
manager.addFields('root', ['FirstName', 'Surname']);
Copy link
Member

Choose a reason for hiding this comment

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

In terms of API, it feels a bit weird to have "root" as a mandated argument. I would've done the function signature the other way around. addFields(fields, path?), just assume root, and start deeper paths there (foo/bar rather than root/foo/bar)

manager.addFields('root', ['Avatar', []]);
manager.addField('root/Avatar', 'URL');

const fields = manager.getConfig().fields;
expect(Array.isArray(fields)).toBe(true);
expect(fields).toContain('FirstName');
expect(fields).toContain('Surname');
expect(fields).toContain('Avatar');
const i = fields.indexOf('Avatar') + 1;
expect(Array.isArray(fields[i])).toBe(true);
expect(fields[i]).toContain('URL');
expect(() => manager.addFields('root/Surname', 'Foo')).toThrow();
});

it('adds args to fields', () => {
const manager = createMock({ pagination: false });
manager.addFields('root', ['FirstName', 'Surname']);
manager.addFields('root', ['Avatar', []]);
manager.addField('root/Avatar', 'URL');

manager.addArg('root/FirstName', 'Test', 'SomeVar');
manager.addArgs('root/Avatar/URL', {
Relative: 'SomeBool',
});
// eslint-disable-next-line no-unused-expressions
manager.setTemplate`query test { ${getFields} }`;

const AST = manager.getGraphqlAST();

expect(AST.kind).toBe('Document');
const compiledQuery = AST.loc.source.body;
expect(compiledQuery).toMatch(/FirstName\s*\(Test:\s*\$SomeVar\s*\)/);
expect(compiledQuery).toMatch(/URL\s*\(Relative:\s*\$SomeBool\s*\)/);
});

it('uses fragments', () => {
const manager = createMock();
manager.useFragment('testFragment');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ describe('Dynamic graphql injection', () => {
'test-transform',
(updater) => {
updater.test('MyTestReadQuery', (manager) => {
manager.addField('Plates');
manager.addField('root', 'Plates');
manager.transformApolloConfig('props', () => (previous) => ({
...previous,
qux: 'baz'
Expand Down Expand Up @@ -119,7 +119,7 @@ describe('Dynamic graphql injection', () => {
'test-transform',
(updater) => {
updater.test('MyTestReadOneQuery', (manager) => {
manager.addField('Plates');
manager.addField('root', 'Plates');
});
}
);
Expand Down Expand Up @@ -151,7 +151,7 @@ describe('Dynamic graphql injection', () => {
'test-transform',
(updater) => {
updater.test('MyTestUpdateQuery', (manager) => {
manager.addField('Plates');
manager.addField('root', 'Plates');
});
}
);
Expand Down Expand Up @@ -182,7 +182,7 @@ describe('Dynamic graphql injection', () => {
'test-transform',
(updater) => {
updater.test('MyTestDeleteQuery', (manager) => {
manager.addField('ID');
manager.addField('root', 'ID');
});
}
);
Expand Down Expand Up @@ -214,7 +214,7 @@ describe('Dynamic graphql injection', () => {
'test-transform',
(updater) => {
updater.test('MyTestCreateQuery', (manager) => {
manager.addField('Claws');
manager.addField('root', 'Claws');
});
}
);
Expand Down