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.

Sign up for GitHub

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

fix: swift file names #713

Merged
merged 4 commits into from
Sep 25, 2023
Merged

fix: swift file names #713

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0786cb0
fix: swift file names
dpilch Sep 25, 2023
f8b1baf
test: update expected test
dpilch Sep 25, 2023
532ebbb
test: legacy generate path
dpilch Sep 25, 2023
934b3cc
style: reuse outputPath
dpilch Sep 25, 2023
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
73 changes: 40 additions & 33 deletions packages/amplify-codegen/src/commands/types.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ const constants = require('../constants');
const { loadConfig } = require('../codegen-config');
const { ensureIntrospectionSchema, getFrontEndHandler, getAppSyncAPIDetails, getAppSyncAPIInfoFromProject } = require('../utils');
const { generateTypes: generateTypesHelper } = require('@aws-amplify/graphql-generator');
const { extractDocumentFromJavascript } = require('@aws-amplify/graphql-types-generator');
const { generate, extractDocumentFromJavascript } = require('@aws-amplify/graphql-types-generator');

async function generateTypes(context, forceDownloadSchema, withoutInit = false, decoupleFrontend = '') {
let frontend = decoupleFrontend;
Expand Down Expand Up @@ -57,25 +57,24 @@ async function generateTypes(context, forceDownloadSchema, withoutInit = false,
const target = cfg.amplifyExtension.codeGenTarget;

const excludes = cfg.excludes.map(pattern => `!${pattern}`);
const queryFiles = glob
.sync([...includeFiles, ...excludes], {
cwd: projectPath,
absolute: true,
})
.map(queryFilePath => {
const fileContents = fs.readFileSync(queryFilePath, 'utf8');
if (
queryFilePath.endsWith('.jsx') ||
queryFilePath.endsWith('.js') ||
queryFilePath.endsWith('.tsx') ||
queryFilePath.endsWith('.ts')
) {
return extractDocumentFromJavascript(fileContents, '');
}
return fileContents;
});
const queryFilePaths = glob.sync([...includeFiles, ...excludes], {
cwd: projectPath,
absolute: true,
});
const queryFiles = queryFilePaths.map(queryFilePath => {
const fileContents = fs.readFileSync(queryFilePath, 'utf8');
if (
queryFilePath.endsWith('.jsx') ||
queryFilePath.endsWith('.js') ||
queryFilePath.endsWith('.tsx') ||
queryFilePath.endsWith('.ts')
) {
return extractDocumentFromJavascript(fileContents, '');
}
return fileContents;
});
if (queryFiles.length === 0) {
throw new Error('No queries found to generate types for, you may need to run \'codegen statements\' first');
throw new Error("No queries found to generate types for, you may need to run 'codegen statements' first");
}
const queries = queryFiles.join('\n');

Expand All @@ -96,22 +95,30 @@ async function generateTypes(context, forceDownloadSchema, withoutInit = false,
const introspection = path.extname(schemaPath) === '.json';

try {
const output = await generateTypesHelper({
schema,
queries,
target,
introspection,
});
const outputs = Object.entries(output);

const outputPath = path.join(projectPath, generatedFileName);
if (outputs.length === 1) {
const [[, contents]] = outputs;
fs.outputFileSync(path.resolve(outputPath), contents);
if (target === 'swift' && fs.existsSync(outputPath) && fs.statSync(outputPath).isDirectory()) {
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the previous behavior for setting multiple files. https://github.com/aws-amplify/amplify-codegen/blob/9f11cfe396bacc603df84233a1b75feabcadaae6/packages/graphql-types-generator/src/generate.ts#L47C35-L47C102

generate(queryFilePaths, schemaPath, outputPath, '', target, '', {
addTypename: true,
complexObjectSupport: 'auto',
});
Copy link
Contributor

Choose a reason for hiding this comment

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

doesn't the new generateTypesHelper handle multiple swift file generation yet?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is bugged. The generateTypesHelper does not set the correct filenames for the generated types. It will require a more complex fix. This solution will fix current production.

} else {
outputs.forEach(([filepath, contents]) => {
fs.outputFileSync(path.resolve(path.join(outputPath, filepath)), contents);
const output = await generateTypesHelper({
schema,
queries,
target,
introspection,
multipleSwiftFiles: false,
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this flag still needed for the current PR? Looks like the change of consuming this value is not included

Copy link
Member Author

Choose a reason for hiding this comment

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

This is necessary to ensure that multiple Swift files are not used in the graphql-generator package.

Copy link
Member Author

Choose a reason for hiding this comment

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

This is currently defaulted to true.

amplify-codegen/packages/graphql-generator/src/types.ts

Line 6 in 756c375

const { schema, target, queries, multipleSwiftFiles = true, introspection = false } = options;

});
const outputs = Object.entries(output);

const outputPath = path.join(projectPath, generatedFileName);
if (outputs.length === 1) {
const [[, contents]] = outputs;
fs.outputFileSync(path.resolve(outputPath), contents);
} else {
outputs.forEach(([filepath, contents]) => {
fs.outputFileSync(path.resolve(path.join(outputPath, filepath)), contents);
});
}
}
codeGenSpinner.succeed(`${constants.INFO_MESSAGE_CODEGEN_GENERATE_SUCCESS} ${path.relative(path.resolve('.'), outputPath)}`);
} catch (err) {
Expand Down
28 changes: 28 additions & 0 deletions packages/amplify-codegen/tests/commands/types.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const { sync } = require('glob-all');
const path = require('path');
const { generateTypes: generateTypesHelper } = require('@aws-amplify/graphql-generator');
const { generate: legacyGenerate } = require('@aws-amplify/graphql-types-generator');
const fs = require('fs-extra');

const { loadConfig } = require('../../src/codegen-config');
Expand All @@ -20,6 +21,7 @@ const MOCK_CONTEXT = {

jest.mock('glob-all');
jest.mock('@aws-amplify/graphql-generator');
jest.mock('@aws-amplify/graphql-types-generator');
jest.mock('../../src/codegen-config');
jest.mock('../../src/utils');
jest.mock('fs-extra');
Expand Down Expand Up @@ -81,9 +83,35 @@ describe('command - types', () => {
schema: 'schema',
target: 'TYPE_SCRIPT_OR_FLOW_OR_ANY_OTHER_LANGUAGE',
introspection: false,
multipleSwiftFiles: false,
});
});

it('should use legacy types generation when generating multiple swift files', async () => {
MOCK_PROJECT.amplifyExtension.codeGenTarget = 'swift';
MOCK_PROJECT.amplifyExtension.generatedFileName = 'typesDirectory';
const forceDownload = false;
fs.readFileSync
.mockReturnValueOnce('query 1')
.mockReturnValueOnce('query 2')
.mockReturnValueOnce('schema');
fs.existsSync.mockReturnValueOnce(true);
fs.statSync.mockReturnValueOnce({
isDirectory: jest.fn().mockReturnValue(true),
});
await generateTypes(MOCK_CONTEXT, forceDownload);
expect(generateTypesHelper).not.toHaveBeenCalled();
expect(legacyGenerate).toHaveBeenCalledWith(
['q1.gql', 'q2.gql'],
'MOCK_PROJECT_ROOT/INTROSPECTION_SCHEMA.JSON',
'MOCK_PROJECT_ROOT/typesDirectory',
'',
'swift',
'',
{ addTypename: true, complexObjectSupport: 'auto' },
);
});

it('should not generate type if the frontend is android', async () => {
const forceDownload = false;
getFrontEndHandler.mockReturnValue('android');
Expand Down