Improve string delimiter escaping in schemaDataModelJSON
#66
Conversation
Wrap the JSON in single quotes instead of backticks to reduce the escape sequence surface area. Escape any single quotes in the stringified JSON. This handles single and double quotes and backticks in the input schema's scalar, field/type/etc. descriptions. - add test `Case03.02` to check output for schema with string delimiters - update other `outputReference` resolvers - add templates to lint check - fix resulting ESLint errors in templates - exclude output from ESLint rules (hide errors for those files when using editor integrations) copied changes from aws#61: - fix query result tests that were sensitive to object key order (use `toEqual` instead of strict JSON string equality) - also check output file size of `.cjs` resolver in test `Case01.02` - fix running test `Case01.04` (use `.cjs` extension) closes aws#65
|
Hi @danielfinke hoping to review this sometime this week |
| @@ -15,12 +15,13 @@ import { loggerError } from "./logger.js"; | |||
|
|
|||
| function resolverJS (schemaModel, queryLanguage, queryClient, __dirname) { | |||
| let code = ''; | |||
| const queryDataModelJSON = JSON.stringify(schemaModel, null, 2); | |||
There was a problem hiding this comment.
Hi @danielfinke, I wanted to ask about the reasoning for dropping the space argument from the call to JSON.stringify(). I think there is some value lost in the legibility of the formatted output.
There was a problem hiding this comment.
@Cole-Greer this is a caveat of using ' as the delimiter: multi-line strings are not available
|
@danielfinke I am curious about the driving factor of this PR. Is it to resolve ESLint issues or did you find a scenario which produced a graphQL schema that wasn't working correctly with the current escaping logic? |
|
@andreachild It was for a schema that would break the module - the JS syntax would become invalid. I have an example schema in #65. @andreachild @Cole-Greer I think a better optional overall, if it acceptable, would be writing the schema data model JSON to a bundled |
…-escaping-in-schemaDataModelJSON
I very much like your idea of moving the schema to its own file and would prefer to skip changing the escaping logic as an intermediate step. |
Issue #, if available: #65
Description of changes:
Wrap the JSON in single quotes instead of backticks to reduce the escape sequence surface area. Escape any single quotes in the stringified JSON. This handles single and double quotes and backticks in the input schema's scalar, field/type/etc. descriptions.
Case03.02to check output for schema with string delimitersoutputReferenceresolverscopied changes from #61:
toEqualinstead of strict JSON string equality).cjsresolver in testCase01.02Case01.04(use.cjsextension)By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.