diff --git a/query-planner-js/src/__tests__/buildPlan.test.ts b/query-planner-js/src/__tests__/buildPlan.test.ts index e677a5584..7535d6d89 100644 --- a/query-planner-js/src/__tests__/buildPlan.test.ts +++ b/query-planner-js/src/__tests__/buildPlan.test.ts @@ -1129,7 +1129,74 @@ describe('fetch operation names', () => { } `); const fetch = ((plan.node as SequenceNode).nodes[1] as FlattenNode).node as FetchNode; - expect(fetch.operation).toMatch(/^query myOp__non_graphqlname__1.*/i); + expect(fetch.operation).toMatch(/^query myOp__non_graphql_name__1.*/i); + }); + + test('ensures sanitization applies repeatedly', () => { + const subgraph1 = { + name: 'S1', + typeDefs: gql` + type Query { + t: T + } + + type T @key(fields: "id") { + id: ID! + } + ` + } + + const subgraph2 = { + name: 'a-na&me-with-plen&ty-replace*ments', + typeDefs: gql` + type T @key(fields: "id") { + id: ID! + x: Int + } + ` + } + + const [api, queryPlanner] = composeAndCreatePlanner(subgraph1, subgraph2); + const operation = operationFromDocument(api, gql` + query myOp { + t { + x + } + } + `); + + const plan = queryPlanner.buildQueryPlan(operation); + expect(plan).toMatchInlineSnapshot(` + QueryPlan { + Sequence { + Fetch(service: "S1") { + { + t { + __typename + id + } + } + }, + Flatten(path: "t") { + Fetch(service: "a-na&me-with-plen&ty-replace*ments") { + { + ... on T { + __typename + id + } + } => + { + ... on T { + x + } + } + }, + }, + }, + } + `); + const fetch = ((plan.node as SequenceNode).nodes[1] as FlattenNode).node as FetchNode; + expect(fetch.operation).toMatch(/^query myOp__a_name_with_plenty_replacements__1.*/i); }); test('handle very non-graph subgraph name', () => { diff --git a/query-planner-js/src/buildPlan.ts b/query-planner-js/src/buildPlan.ts index aecf953e9..732e69f2a 100644 --- a/query-planner-js/src/buildPlan.ts +++ b/query-planner-js/src/buildPlan.ts @@ -628,8 +628,8 @@ function toValidGraphQLName(subgraphName: string): string { // Note that this could theoretically lead to substantial changes to the name but should // work well in practice (and if it's a huge problem for someone, we can change it). const sanitized = subgraphName - .replace('-', '_') - .replace(/[^_0-9A-Za-z]/i, ''); + .replace(/-/ig, '_') + .replace(/[^_0-9A-Za-z]/ig, ''); return sanitized.match(/^[0-9].*/i) ? '_' + sanitized : sanitized; }