Skip to content

Commit

Permalink
fix: post-process cfn files for minification, consolidating concern o…
Browse files Browse the repository at this point in the history
…f minification a bit
  • Loading branch information
alharris-at committed Feb 22, 2023
1 parent 73eb14a commit 203a46e
Show file tree
Hide file tree
Showing 4 changed files with 124 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
import { minifyJSONFile, minifyAllJSONInFolderRecursively } from '../../utils/minify-utils'
import path from 'path';
import fs from 'fs-extra';
import os from 'os';

const JSON_STRING = `{
"city": "Seattle",
"state": "WA"
}`;

const NON_JSON_STRING = `CITY = Seattle
STATE = WA
`;

describe('minifyJSONFile', () => {
const writeFileMinifyAndReturnReadContents = (filePath: string, contents: string): string => {
const tmpFilePath = path.join(os.tmpdir(), filePath);
fs.writeFileSync(tmpFilePath, contents);
minifyJSONFile(tmpFilePath);
return fs.readFileSync(tmpFilePath, 'utf-8');
};

it('minifies JSON files', () => {
const minifiedContents = writeFileMinifyAndReturnReadContents('testfile.json', JSON_STRING);
expect(minifiedContents.length).toBeLessThan(JSON_STRING.length);
});


it('does not change a non-json file with json contents', () => {
const minifiedContents = writeFileMinifyAndReturnReadContents('testfile.dat', JSON_STRING);
expect(minifiedContents.length).toEqual(JSON_STRING.length);
});

it('does not change a non-json file with non-json contents', () => {
const minifiedContents = writeFileMinifyAndReturnReadContents('testfile.env', NON_JSON_STRING);
expect(minifiedContents.length).toEqual(NON_JSON_STRING.length);
});
});

describe('minifyAllJSONInFolderRecursively', () => {
it('will minify json files, but leave non-json files alone', () => {
// This test will set up a directory structure as follows
// .
// ├── file.json
// ├── config.env
// └── nested_stacks
// ├── nested.env
// ├── nested_1.json
// ├── nested_2.json
// └── sub_nested_stacks
// ├── sub_nested.env
// └── sub_nested.json
// We will then ensure that after a single invocation file.json, nested_1.json, nested_2.json, and sub_nested.json are all minified
// and that config.env is the same length.
const testDirectory = path.join(os.tmpdir(), 'minifyTestDir');
const nestedStacksDirectory = path.join(testDirectory, 'nested_stacks');
const subNestedStacksDirectory = path.join(nestedStacksDirectory, 'sub_nested_stacks');

// Clean and setup directory structure
fs.removeSync(testDirectory);
fs.mkdirSync(testDirectory);
fs.mkdirSync(nestedStacksDirectory);
fs.mkdirSync(subNestedStacksDirectory);

// Write files
fs.writeFileSync(path.join(testDirectory, 'file.json'), JSON_STRING);
fs.writeFileSync(path.join(testDirectory, 'config.env'), NON_JSON_STRING);
fs.writeFileSync(path.join(nestedStacksDirectory, 'nested_1.json'), JSON_STRING);
fs.writeFileSync(path.join(nestedStacksDirectory, 'nested_2.json'), JSON_STRING);
fs.writeFileSync(path.join(nestedStacksDirectory, 'nested.env'), NON_JSON_STRING);
fs.writeFileSync(path.join(subNestedStacksDirectory, 'sub_nested.json'), JSON_STRING);
fs.writeFileSync(path.join(subNestedStacksDirectory, 'sub_nested.env'), NON_JSON_STRING);

// Apply recursive minification
minifyAllJSONInFolderRecursively(testDirectory);

// Verify all `.json` files are minified, and `.env` files are not.
expect(fs.readFileSync(path.join(testDirectory, 'file.json')).length).toBeLessThan(JSON_STRING.length);
expect(fs.readFileSync(path.join(testDirectory, 'config.env')).length).toEqual(NON_JSON_STRING.length);
expect(fs.readFileSync(path.join(nestedStacksDirectory, 'nested_1.json')).length).toBeLessThan(JSON_STRING.length);
expect(fs.readFileSync(path.join(nestedStacksDirectory, 'nested_2.json')).length).toBeLessThan(JSON_STRING.length);
expect(fs.readFileSync(path.join(nestedStacksDirectory, 'nested.env')).length).toEqual(NON_JSON_STRING.length);
expect(fs.readFileSync(path.join(subNestedStacksDirectory, 'sub_nested.json')).length).toBeLessThan(JSON_STRING.length);
expect(fs.readFileSync(path.join(subNestedStacksDirectory, 'sub_nested.env')).length).toEqual(NON_JSON_STRING.length);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ import { prePushTemplateDescriptionHandler } from './template-description-utils'
import { buildOverridesEnabledResources } from './build-override-enabled-resources';

import { invokePostPushAnalyticsUpdate } from './plugin-client-api-analytics';
import { minifyJSONFile } from './utils/minify-utils';

const logger = fileLogger('push-resources');

Expand Down Expand Up @@ -767,6 +768,9 @@ export const uploadTemplateToS3 = async (
amplifyMeta: $TSMeta,
): Promise<void> => {
const cfnFile = path.parse(filePath).base;
if (context.input.options?.minify) {

This comment has been minimized.

Copy link
@sobolk

sobolk Feb 22, 2023

Member

do we have at least one e2e test that's using this flag ? If not we should add one that hits this if and the other one in upload appsync files.

minifyJSONFile(filePath);
}
const s3 = await S3.getInstance(context);

const s3Params = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ const path = require('path');
const TransformPackage = require('graphql-transformer-core');
const { S3 } = require('./aws-utils/aws-s3');
const { fileLogger } = require('./utils/aws-logger');
const { minifyAllJSONInFolderRecursively } = require('./utils/minify-utils');

const logger = fileLogger('upload-appsync-files');

const ROOT_APPSYNC_S3_KEY = 'amplify-appsync-files';
Expand Down Expand Up @@ -111,7 +113,7 @@ async function uploadAppSyncFiles(context, resourcesToUpdate, allResources, opti
if (personalParams.CreateAPIKey !== undefined && personalParams.APIKeyExpirationEpoch !== undefined) {
context.print.warning(
'APIKeyExpirationEpoch and CreateAPIKey parameters should not used together because it can cause ' +
'unwanted behavior. In the future APIKeyExpirationEpoch will be removed, use CreateAPIKey instead.',
'unwanted behavior. In the future APIKeyExpirationEpoch will be removed, use CreateAPIKey instead.',
);
}

Expand All @@ -125,7 +127,7 @@ async function uploadAppSyncFiles(context, resourcesToUpdate, allResources, opti

context.print.warning(
"APIKeyExpirationEpoch parameter's -1 value is deprecated to disable " +
'the API Key creation. In the future CreateAPIKey parameter replaces this behavior.',
'the API Key creation. In the future CreateAPIKey parameter replaces this behavior.',
);
} else {
currentParameters.CreateAPIKey = 1;
Expand Down Expand Up @@ -189,6 +191,9 @@ async function uploadAppSyncFiles(context, resourcesToUpdate, allResources, opti
if (!fs.existsSync(resourceBuildDir)) {
return;
}
if (context.input.options?.minify) {
minifyAllJSONInFolderRecursively(resourceBuildDir);
}
const spinner = new ora('Uploading files.');
spinner.start();
await TransformPackage.uploadAPIProject({
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import * as fs from 'fs-extra';
import * as path from 'path';

/**
* Given a path to a json file, minify that file by reading in as JSON
* and writing back out without any whitespace.
* @param jsonFilePath the path to the JSON file we are going to minify.
* @returns when the file has been minified and written back to disk.
*/
export const minifyJSONFile = (jsonFilePath: string): void => {
if (!jsonFilePath.includes('.json')) return; // Don't convert files not ending in `.json`
const originalJSON = fs.readFileSync(jsonFilePath, 'utf-8');
const minifiedJSON = JSON.stringify(JSON.parse(originalJSON));
fs.writeFileSync(jsonFilePath, minifiedJSON);
};

/**
* Recursively walk a folder, and minify (print without whitespace) all the json files you discover
* @param rootPath the top of the tree to walk
*/
export const minifyAllJSONInFolderRecursively = (rootPath: string): void => {
fs.readdirSync(rootPath).forEach(childHandle => {
const childPath = path.join(rootPath, childHandle);
if (fs.lstatSync(childPath).isDirectory()) minifyAllJSONInFolderRecursively(childPath);
if (fs.lstatSync(childPath).isFile() && childPath.includes('.json')) minifyJSONFile(childPath);
});
};

This comment has been minimized.

Copy link
@sobolk

sobolk Feb 22, 2023

Member

Two things here:

  1. I'd rename this file to minify-json.ts, we already know it's in utils dir.
  2. We now enforce couple of eslint and prettier rules, this file is definietly missing new line.

See here for commands

- run: yarn eslint . --ext .js,.jsx,.ts,.tsx --max-warnings 1085
- run: yarn eslint --no-eslintrc --config .eslint.package.json.js "**/package.json"
- run: yarn prettier --check .
.
(there's also PR to create aliases in main package.json script, so this will get easier #12066 )

0 comments on commit 203a46e

Please sign in to comment.