Skip to content

Commit

Permalink
Fix corrupt images (#55)
Browse files Browse the repository at this point in the history
There is an issue where the `assets` images were all corrupted in the
generated Belt app. This is occurring because Belt reads files as utf-8
text and then writes them when copying. It isn't handling binary files
appropriately. The only binary files I’m aware of right now are in
`templates/boilerplate/assets`.

To reproduce the original issue, from Belt project dir:

```
node bin/belt.js NewApp
cd builds/NewApp
open assets/splash.png
# corrupted image
```

This updates the `copyTemplateDirectory` function to use `fs.copy` for
binary files (currently defined using an array of filenames that we
identify as binary, but we might want something more robust in the
future) instead of reading the file, optionally transforming it, and
then writing it.

---------

Co-authored-by: Diego Oliveira <[email protected]>
  • Loading branch information
Stephen Hanson and codeofdiego authored Aug 27, 2024
1 parent 41fc31b commit 8ad7619
Show file tree
Hide file tree
Showing 9 changed files with 592 additions and 358 deletions.
5 changes: 1 addition & 4 deletions __mocks__/fs-extra.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,6 @@ export default {

return memfs.readFileSync(file, options);
},
readFile(path, options) {
return Promise.resolve(this.readFileSync(path, options));
},
outputFile: async (file, data, options) => {
const dirname = path.dirname(file);
const exists = memfs.existsSync(dirname);
Expand Down Expand Up @@ -68,8 +65,8 @@ export default {
// read templates from actual fs
const sourceFS = dontMock(src) ? fse : memfs;

memfs.mkdirSync(path.dirname(dest), { recursive: true });
if (sourceFS.existsSync(src) && sourceFS.statSync(src).isDirectory(src)) {
memfs.mkdirSync(dest, { recursive: true });
sourceFS.readdirSync(src).forEach((childItemName) => {
this.copySync(
path.join(src, childItemName),
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@
"npm-run-all": "^4.1.5",
"tsup": "^7.2.0",
"typescript": "^5.0.4",
"vitest": "^0.34.1"
"vitest": "^2.0.5"
},
"eslintConfig": {
"extends": [
Expand Down
4 changes: 2 additions & 2 deletions src/commands/__tests__/createApp.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ test('prompts for app name if not supplied', async () => {
test('exits if directory already exists', async () => {
(print as Mock).mockReset();
vi.spyOn(process, 'exit');
process.exit = vi.fn();
process.exit = vi.fn<typeof process.exit>();

vol.fromJSON({ 'MyApp/package.json': '{}' }, './');

Expand All @@ -67,7 +67,7 @@ test('converts directory to camel case and strips special characters', async ()
test('exits if app name does not start with a letter', async () => {
(print as Mock).mockReset();
vi.spyOn(process, 'exit');
process.exit = vi.fn();
process.exit = vi.fn<typeof process.exit>();
vol.fromJSON({ 'MyApp/package.json': '{}' }, './');

await createApp('555MyApp');
Expand Down
2 changes: 2 additions & 0 deletions src/commands/__tests__/notifications.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@ import { Mock, expect, test, vi } from 'vitest';
import addDependency from '../../util/addDependency';
import { addNotifications } from '../notifications';

vi.mock('../../util/print', () => ({ default: vi.fn() }));

vi.mock('@inquirer/prompts', () => ({
input: vi.fn(),
confirm: vi.fn(),
Expand Down
58 changes: 32 additions & 26 deletions src/commands/createApp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,44 +29,43 @@ export async function createApp(
) {
const { interactive = true } = options;
globals.interactive = interactive;

const appName = await validateAndSanitizeAppName(name);

await ensureDirectoryDoesNotExist(appName);
await printIntro(appName);

const spinner = ora('Creating app with Belt').start();

await exec(`mkdir ${appName}`);

await copyTemplateDirectory({
templateDir: 'boilerplate',
destinationDir: appName,
gitignore: await boilerplateIgnoreFiles(),
stringSubstitutions: {
'app.json': {
BELT_APP_NAME: appName,
try {
await exec(`mkdir ${appName}`);

await copyTemplateDirectory({
templateDir: 'boilerplate',
destinationDir: appName,
gitignore: await boilerplateIgnoreFiles(),
stringSubstitutions: {
'app.json': {
BELT_APP_NAME: appName,
},
'package.json': {
belt_app_name: _.kebabCase(appName),
},
},
'package.json': {
belt_app_name: _.kebabCase(appName),
},
},
});
});

spinner.succeed('Created new Belt app with Expo');
spinner.succeed('Created new Belt app with Expo');

process.chdir(`./${appName}`);
process.chdir(`./${appName}`);

spinner.start('Installing dependencies');
const packageManager = getPackageManager(options);
await exec(`${packageManager} install`);
await exec('git init');
await commit('Initial commit');
spinner.succeed('Installed dependencies');
spinner.start('Installing dependencies');
const packageManager = getPackageManager(options);
await exec(`${packageManager} install`);
await exec('git init');
await commit('Initial commit');
spinner.succeed('Installed dependencies');

print(chalk.green(`\n\n👖 ${appName} successfully configured!`));
print(chalk.green(`\n\n👖 ${appName} successfully configured!`));

print(`
print(`
Your pants are now secure! To get started with your new app:
cd ${appName}
Expand All @@ -75,6 +74,13 @@ ${packageManager} run android
For more information about Belt, visit https://github.com/thoughtbot/belt.
`);
} catch (e) {
spinner.fail('An error occurred creating the app\n');
if (e instanceof Error) {
print(chalk.red(e.message));
}
process.exit(1);
}
}

/**
Expand Down
2 changes: 1 addition & 1 deletion src/util/__tests__/validateAndSanitizeAppName.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ describe('validateAndSanitizeAppName', () => {
test('returns a warning when the application name does not start with a letter', async () => {
(print as Mock).mockReset();
vi.spyOn(process, 'exit');
process.exit = vi.fn();
process.exit = vi.fn<typeof process.exit>();
(input as Mock).mockReturnValue('123MyApp');
await validateAndSanitizeAppName(undefined);

Expand Down
70 changes: 53 additions & 17 deletions src/util/copyTemplateDirectory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,15 @@ import path from 'path';
import { PACKAGE_ROOT } from '../constants';
import writeFile from './writeFile';

const UNSUPPORTED_EXTENSIONS = [
'.png',
'.jpg',
'.jpeg',
'.gif',
'.svg',
'.pdf',
];

type Substitutions = {
[fileNamePattern: string]: Record<string, string>;
};
Expand Down Expand Up @@ -49,32 +58,59 @@ export default async function copyTemplateDirectory({
const filenames = await getFiles(srcDir, gitignore);
await Promise.all(
filenames.map(async (filename) => {
const destinationFilename = path.join(
destinationDir,
path.relative(srcDir, filename),
);

let contents = (await fs.readFile(filename)).toString();
const substitutions = substitutionsForFile(filename, stringSubstitutions);
if (Object.keys(substitutions).length > 0) {
contents = Object.entries(substitutions).reduce((acc, [key, value]) => {
return acc.replaceAll(new RegExp(key, 'g'), value);
}, contents);
const relativeFilename = path.relative(srcDir, filename);
const destinationFilename = path.join(destinationDir, relativeFilename);

if (substitutionsSupported(filename)) {
const contents = await makeFileSubstitutions(
filename,
stringSubstitutions,
variables,
);

return writeFile(destinationFilename.replace(/\.eta$/, ''), contents);
}

if (filename.endsWith('.eta')) {
contents = eta.render(contents, variables);
try {
// file is binary or otherwise doesn't support substitutions, just copy
// it over instead of reading it to a string
return await fs.copy(filename, destinationFilename);
} catch (e) {
if (e instanceof Error) {
throw new Error(
`An error occurred copying file ${relativeFilename} to ${destinationFilename}: ${e.message}`,
);
}
}

return writeFile(destinationFilename.replace(/\.eta$/, ''), contents);
return null;
}),
);
}

async function makeFileSubstitutions(
filename: string,
stringSubstitutions: Substitutions,
variables: object,
): Promise<string> {
let contents = (await fs.readFile(filename)).toString();
const substitutions = substitutionsForFile(filename, stringSubstitutions);
if (Object.keys(substitutions).length > 0) {
contents = Object.entries(substitutions).reduce((acc, [key, value]) => {
return acc.replaceAll(new RegExp(key, 'g'), value);
}, contents);
}

if (filename.endsWith('.eta')) {
contents = eta.render(contents, variables);
}

return contents;
}

function substitutionsSupported(filename: string) {
const unsupportedExtensions = ['png', 'jpg', 'jpeg', 'gif', 'svg', 'pdf'];
const extension = filename.split('.').pop()?.toLocaleLowerCase();
return !extension || !unsupportedExtensions.includes(extension);
const extension = path.extname(filename).toLocaleLowerCase();
return !extension || !UNSUPPORTED_EXTENSIONS.includes(extension);
}

function substitutionsForFile(
Expand Down
4 changes: 3 additions & 1 deletion src/util/print/__tests__/copyTemplateDirectory.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,20 @@ import copyTemplateDirectory from '../../copyTemplateDirectory';

vi.mock('../../print', () => ({ default: vi.fn() }));

test('copies directory structure to destination', async () => {
test('copies directory structure to destination, including binary files (based on the extension)', async () => {
fse.mockTemplates();
const json = {
'templates/testing/jest.config.js': '1',
'templates/testing/src/test/render.ts': '2',
'templates/testing/assets/splash.png': '3',
};
vol.fromJSON(json, './');

await copyTemplateDirectory({ templateDir: 'testing', destinationDir: '.' });

expect(fs.readFileSync('jest.config.js', 'utf8')).toEqual('1');
expect(fs.readFileSync('src/test/render.ts', 'utf8')).toEqual('2');
expect(fs.readFileSync('assets/splash.png', 'utf8')).toEqual('3');
});

test('compiles files with .eta file extensions', async () => {
Expand Down
Loading

0 comments on commit 8ad7619

Please sign in to comment.