Skip to content

Commit

Permalink
Makes flow generated '%future added value' for enums optional
Browse files Browse the repository at this point in the history
Summary:
As suggested by leebyron in #2351, this adds an option to the `relay-compiler`  so it doesn't add `%future added value` to each GraphQL generated enum. My reasoning is that you might have a strict control over your GraphQL service, so you'd have to handle any future value by rerunning the compiler, with the new version of the schema.

I'm not totally convinced by the name `strictFlowTypes`, but I don't really have much better now. My thought between this name is that the option would be a generic option that configures the generated flow types, and would not be specifically to enums.

Regarding test cases, I'm not sure of how I should tackle that, since the only tests are snapshots, and there are no tests with different configurations of the `RelayFlowGenerator`. I could however try to make that happen if you'd like!
Closes #2368

Reviewed By: kassens

Differential Revision: D7343235

Pulled By: jstejada

fbshipit-source-id: a74bbea48708a1b432fdcc7267fbba5efd2b950e
  • Loading branch information
Augustin Le Fèvre authored and facebook-github-bot committed Apr 3, 2018
1 parent 3002e97 commit 1e87e43
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 24 deletions.
14 changes: 12 additions & 2 deletions packages/relay-compiler/bin/RelayCompilerBin.js
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ async function run(options: {
watch?: ?boolean,
validate: boolean,
quiet: boolean,
noFutureProofEnums: boolean,
}) {
const schemaPath = path.resolve(process.cwd(), options.schema);
if (!fs.existsSync(schemaPath)) {
Expand Down Expand Up @@ -162,7 +163,7 @@ Ensure that one such file exists in ${srcDir} or its parents.
};
const writerConfigs = {
js: {
getWriter: getRelayFileWriter(srcDir),
getWriter: getRelayFileWriter(srcDir, options.noFutureProofEnums),
isGeneratedFile: (filePath: string) =>
filePath.endsWith('.js') && filePath.includes('__generated__'),
parser: 'js',
Expand Down Expand Up @@ -193,7 +194,7 @@ Ensure that one such file exists in ${srcDir} or its parents.
}
}

function getRelayFileWriter(baseDir: string) {
function getRelayFileWriter(baseDir: string, noFutureProofEnums: boolean) {
return ({
onlyValidate,
schema,
Expand All @@ -217,6 +218,7 @@ function getRelayFileWriter(baseDir: string) {
inputFieldWhiteListForFlow: [],
schemaExtensions,
useHaste: false,
noFutureProofEnums,
},
onlyValidate,
schema,
Expand Down Expand Up @@ -333,6 +335,14 @@ const argv = yargs
type: 'boolean',
default: false,
},
noFutureProofEnums: {
describe:
'This option controls whether or not a catch-all entry is added to enum type definitions ' +
'for values that may be added in the future. Enabling this means you will have to update ' +
'your application whenever the GraphQL server schema adds new enum values to prevent it ' +
'from breaking.',
default: false,
},
})
.help().argv;

Expand Down
2 changes: 2 additions & 0 deletions packages/relay-compiler/codegen/RelayFileWriter.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ export type WriterConfig = {
platform?: string,
relayRuntimeModule?: string,
schemaExtensions: Array<string>,
noFutureProofEnums: boolean,
useHaste: boolean,
// Haste style module that exports flow types for GraphQL enums.
// TODO(T22422153) support non-haste environments
Expand Down Expand Up @@ -280,6 +281,7 @@ class RelayFileWriter implements FileWriterInterface {
inputFieldWhiteList: this._config.inputFieldWhiteListForFlow,
relayRuntimeModule,
useHaste: this._config.useHaste,
noFutureProofEnums: this._config.noFutureProofEnums,
});

const sourceHash = Profiler.run('hashGraphQL', () =>
Expand Down
12 changes: 10 additions & 2 deletions packages/relay-compiler/core/RelayFlowGenerator.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ type Options = {|
+existingFragmentNames: Set<string>,
+inputFieldWhiteList: $ReadOnlyArray<string>,
+relayRuntimeModule: string,
+noFutureProofEnums: boolean,
|};

export type State = {|
Expand Down Expand Up @@ -240,6 +241,7 @@ function createVisitor(options: Options) {
usedEnums: {},
usedFragments: new Set(),
useHaste: options.useHaste,
noFutureProofEnums: options.noFutureProofEnums,
};

return {
Expand Down Expand Up @@ -442,7 +444,11 @@ function getFragmentImports(state: State) {
return imports;
}

function getEnumDefinitions({enumsHasteModule, usedEnums}: State) {
function getEnumDefinitions({
enumsHasteModule,
usedEnums,
noFutureProofEnums,
}: State) {
const enumNames = Object.keys(usedEnums).sort();
if (enumNames.length === 0) {
return [];
Expand All @@ -453,7 +459,9 @@ function getEnumDefinitions({enumsHasteModule, usedEnums}: State) {
return enumNames.map(name => {
const values = usedEnums[name].getValues().map(({value}) => value);
values.sort();
values.push('%future added value');
if (!noFutureProofEnums) {
values.push('%future added value');
}
return exportType(
name,
t.unionTypeAnnotation(
Expand Down
64 changes: 44 additions & 20 deletions packages/relay-compiler/core/__tests__/RelayFlowGenerator-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,26 +20,50 @@ const parseGraphQLText = require('parseGraphQLText');
const {transformASTSchema} = require('ASTConvert');
const {generateTestsFromFixtures} = require('RelayModernTestUtils');

function generate(text, options) {
const schema = transformASTSchema(RelayTestSchema, [
RelayRelayDirectiveTransform.SCHEMA_EXTENSION,
]);
const {definitions} = parseGraphQLText(schema, text);
return new GraphQLCompilerContext(RelayTestSchema, schema)
.addAll(definitions)
.applyTransforms(RelayFlowGenerator.flowTransforms)
.documents()
.map(doc => RelayFlowGenerator.generate(doc, options))
.join('\n\n');
}

describe('RelayFlowGenerator', () => {
generateTestsFromFixtures(`${__dirname}/fixtures/flow-generator`, text => {
const schema = transformASTSchema(RelayTestSchema, [
RelayRelayDirectiveTransform.SCHEMA_EXTENSION,
]);
const {definitions} = parseGraphQLText(schema, text);
return new GraphQLCompilerContext(RelayTestSchema, schema)
.addAll(definitions)
.applyTransforms(RelayFlowGenerator.flowTransforms)
.documents()
.map(doc =>
RelayFlowGenerator.generate(doc, {
customScalars: {},
enumsHasteModule: null,
existingFragmentNames: new Set(['PhotoFragment']),
inputFieldWhiteList: [],
relayRuntimeModule: 'relay-runtime',
useHaste: true,
}),
)
.join('\n\n');
generateTestsFromFixtures(`${__dirname}/fixtures/flow-generator`, text =>
generate(text, {
customScalars: {},
enumsHasteModule: null,
existingFragmentNames: new Set(['PhotoFragment']),
inputFieldWhiteList: [],
relayRuntimeModule: 'relay-runtime',
useHaste: true,
}),
);

it('does not add `%future added values` when the noFutureProofEnums option is set', () => {
const text = `
fragment ScalarField on User {
traits
}
`;
const types = generate(text, {
customScalars: {},
enumsHasteModule: null,
existingFragmentNames: new Set(['PhotoFragment']),
inputFieldWhiteList: [],
relayRuntimeModule: 'relay-runtime',
useHaste: true,
// This is what's different from the tests above.
noFutureProofEnums: true,
});
// Without the option, PersonalityTraits would be `('CHEERFUL' | ... | '%future added value');`
expect(types).toContain(
"export type PersonalityTraits = ('CHEERFUL' | 'DERISIVE' | 'HELPFUL' | 'SNARKY');",
);
});
});

0 comments on commit 1e87e43

Please sign in to comment.