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.

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: Move GitHub downloads to optimized method #224

Merged
merged 4 commits into from
Jan 21, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
20 changes: 17 additions & 3 deletions lib/__tests__/functions.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import fs, { PathLike } from 'fs-extra';
import findup from 'findup-sync';
import { getCwd } from '../path';
import { downloadGithubRepoContents } from '../github';
import { fetchFileFromRepository as __fetchFileFromRepository } from '../github';
import {
createFunction,
isObjectOrFunction,
Expand All @@ -23,6 +23,11 @@ const mockedFsReadFileSync = fs.readFileSync as jest.MockedFunction<
typeof fs.readFileSync
>;

const fetchFileFromRepository =
__fetchFileFromRepository as jest.MockedFunction<
typeof __fetchFileFromRepository
>;

describe('lib/cms/functions', () => {
describe('createFunction', () => {
const mockFunctionInfo = {
Expand Down Expand Up @@ -60,14 +65,23 @@ describe('lib/cms/functions', () => {
});

it('should create a new function successfully', async () => {
const functionSourceCode = 'function code';
fetchFileFromRepository.mockResolvedValue(functionSourceCode);

await createFunction(mockFunctionInfo, mockDest);

expect(fs.mkdirp).not.toHaveBeenCalled();

expect(downloadGithubRepoContents).toHaveBeenCalledWith(
expect(fetchFileFromRepository).toHaveBeenCalledWith(
'HubSpot/cms-sample-assets',
'functions/sample-function.js',
'/mock/dest/testFolder.functions/testFunction.js'
'main'
);

// Check that the config file was updated
expect(fs.writeFileSync).toHaveBeenCalledWith(
'/mock/dest/testFolder.functions/testFunction.js',
functionSourceCode
);

// Check that the config file was updated
Expand Down
15 changes: 7 additions & 8 deletions lib/__tests__/templates.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,14 @@ import {
isCodedFile,
createTemplate,
} from '../cms/templates';
import { downloadGithubRepoContents as __downloadGithubRepoContents } from '../github';
import { cloneGithubRepo as __cloneGithubRepo } from '../github';

jest.mock('fs-extra');
jest.mock('../github');

const downloadGithubRepoContents =
__downloadGithubRepoContents as jest.MockedFunction<
typeof __downloadGithubRepoContents
>;
const cloneGithubRepo = __cloneGithubRepo as jest.MockedFunction<
typeof __cloneGithubRepo
>;

// eslint-disable-next-line @typescript-eslint/no-explicit-any
const makeAnnotation = (options: { [key: string]: any } = {}) => {
Expand Down Expand Up @@ -63,10 +62,10 @@ describe('lib/cms/templates', () => {
jest.spyOn(fs, 'existsSync').mockReturnValue(false);
await createTemplate('my-template', '/', 'page-template');

expect(downloadGithubRepoContents).toHaveBeenCalledWith(
expect(cloneGithubRepo).toHaveBeenCalledWith(
'HubSpot/cms-sample-assets',
'templates/page-template.html',
'/my-template.html'
'/my-template.html',
{ sourceDir: 'templates/page-template.html' }
);
});
});
Expand Down
8 changes: 5 additions & 3 deletions lib/cms/functions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import fs from 'fs-extra';
import path from 'path';
import findup from 'findup-sync';
import { getCwd } from '../path';
import { downloadGithubRepoContents } from '../github';
import { fetchFileFromRepository } from '../github';
import { logger } from '../logger';
import { i18n } from '../../utils/lang';
import { FileSystemError } from '../../models/FileSystemError';
Expand Down Expand Up @@ -192,12 +192,14 @@ export async function createFunction(
);
}

await downloadGithubRepoContents(
const result = await fetchFileFromRepository(
'HubSpot/cms-sample-assets',
'functions/sample-function.js',
functionFilePath
'main'
);

fs.writeFileSync(functionFilePath, result);

logger.log(
i18n(`${i18nKey}.createFunction.createdFunctionFile`, {
path: functionFilePath,
Expand Down
59 changes: 35 additions & 24 deletions lib/cms/modules.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import path from 'path';
import fs from 'fs-extra';
import { getCwd } from '../path';
import { walk } from '../fs';
import { listGithubRepoContents, downloadGithubRepoContents } from '../github';
import { listGithubRepoContents, cloneGithubRepo } from '../github';
import { logger } from '../logger';
import {
isPathInput,
Expand Down Expand Up @@ -210,26 +210,17 @@ export async function createModule(
);

// Filter out certain fetched files from the response
const moduleFileFilter = (src: string, dest: string) => {
const moduleFileFilter = (src: string) => {
const emailEnabled = moduleDefinition.contentTypes.includes('EMAIL');

switch (path.basename(src)) {
case 'meta.json':
fs.writeJSONSync(dest, moduleMetaData, { spaces: 2 }); // writing a meta.json file to standard HubL modules
return false;
case 'module.js':
case 'module.css':
if (emailEnabled) {
return false;
}
return true;
return !emailEnabled;
case 'global-sample-react-module.css':
case 'stories':
case 'tests':
if (getInternalVersion) {
return true;
}
return false;
return getInternalVersion;
default:
return true;
}
Expand All @@ -240,14 +231,36 @@ export async function createModule(
? 'Sample.module'
: 'SampleReactModule';

await downloadGithubRepoContents(
'HubSpot/cms-sample-assets',
`modules/${sampleAssetPath}`,
destPath,
'',
moduleFileFilter
const sourceDir = `modules/${sampleAssetPath}`;

await cloneGithubRepo('HubSpot/cms-sample-assets', destPath, {
sourceDir,
});

const files = await walk(destPath);

files
.filter(filePath => !moduleFileFilter(filePath))
.forEach(filePath => {
fs.unlinkSync(filePath);
});

if (!getInternalVersion) {
fs.removeSync(path.join(destPath, 'stories'));
fs.removeSync(path.join(destPath, 'tests'));
}

// Get and write the metafiles
const metaFiles = files.filter(
filePath => path.basename(filePath) === 'meta.json'
);

metaFiles.forEach(metaFile => {
fs.writeJSONSync(metaFile, moduleMetaData, {
spaces: 2,
});
});

// Updating React module files after fetch
if (isReactModule) {
await updateFileContents(
Expand All @@ -271,11 +284,9 @@ export async function retrieveDefaultModule(
return defaultReactModules;
}

await downloadGithubRepoContents(
'HubSpot/cms-react',
`default-react-modules/src/components/modules/${name}`,
dest
);
await cloneGithubRepo('HubSpot/cms-react', dest, {
sourceDir: `default-react-modules/src/components/modules/${name}`,
});
}

const MODULE_HTML_EXTENSION_REGEX = new RegExp(
Expand Down
10 changes: 4 additions & 6 deletions lib/cms/templates.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import fs from 'fs-extra';
import path from 'path';
import { downloadGithubRepoContents } from '../github';
import { cloneGithubRepo } from '../github';
import { logger } from '../logger';
import { i18n } from '../../utils/lang';

Expand Down Expand Up @@ -74,11 +74,9 @@ export async function createTemplate(
})
);

await downloadGithubRepoContents(
'HubSpot/cms-sample-assets',
assetPath,
filePath
);
await cloneGithubRepo('HubSpot/cms-sample-assets', filePath, {
sourceDir: assetPath,
});
}

export const TEMPLATE_TYPES = {
Expand Down
5 changes: 4 additions & 1 deletion lib/github.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,10 @@ export async function fetchGitHubRepoContentFromDownloadUrl(
fs.outputFileSync(dest, fileContents);
}

// Writes files from a public repository to the destination folder
/**
* Writes files from a public repository to the destination folder
@deprecated - This method fetches all the files individually, which can hit rate limits for unauthorized requests. Use `cloneGithubRepo` instead.
*/
export async function downloadGithubRepoContents(
repoPath: RepoPath,
contentPath: string,
Expand Down
Loading