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

feat(storefront): bctheme-1064 Modify Stencil bundle for using Components UI Library #904

Merged
merged 1 commit into from
Jun 20, 2022
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
2 changes: 1 addition & 1 deletion lib/regions.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ describe('Stencil Bundle', () => {
});

it('should return all regions with the right order.', async () => {
const assembleTemplatesTask = promisify(bundle.assembleTemplatesTask.bind(bundle));
const assembleTemplatesTask = bundle.assembleTemplatesTask.bind(bundle);
const generateManifest = promisify(bundle.generateManifest.bind(bundle));

const templates = await assembleTemplatesTask();
Expand Down
211 changes: 131 additions & 80 deletions lib/stencil-bundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ class Bundle {
}

this.tasks = tasks;

this._getExternalLibs = this._getExternalLibs.bind(this);
}

/**
Expand Down Expand Up @@ -151,51 +153,97 @@ class Bundle {
};
}

assembleTemplatesTask(callback) {
console.log('Template Parsing Started...');
/**
* helps to find any node modules dependencies with
* ui templates and returns its list
*
* @private
* @param {string} content
* @returns {Array}
*/
async _getExternalLibs(templatePath) {
const content = await fs.promises.readFile(templatePath, { encoding: 'utf-8' });
const externalPathRegex = /{{2}>\s*(external)[^{]*?}{2}/g;
const externalTemplatesImports = content.match(externalPathRegex);

recursiveReadDir(this.templatesPath, ['!*.html'], (readdirError, files) => {
if (readdirError) {
return callback(readdirError);
}
if (!externalTemplatesImports) return [];

const partials = files.map((file) => {
return upath.toUnix(
file.replace(this.templatesPath + path.sep, '').replace(/\.html$/, ''),
);
});
return externalTemplatesImports.map((templateImport) => templateImport.split('/')[1]);
}

return async.map(
partials,
templateAssembler.assembleAndBundle.bind(null, this.templatesPath),
(err, results) => {
const ret = {};
async assembleTemplatesTask(callback) {
console.log('Template Parsing Started...');
const internalTemplatesList = await recursiveReadDir(this.templatesPath, ['!*.html']);
let externalLibs;

if (err) {
return callback(err);
}
try {
const removeDuplicates = (arr) => Array.from(new Set(arr.flat()));
const temp = internalTemplatesList.map(this._getExternalLibs);
const result = await Promise.all(temp);

partials.forEach((file, index) => {
ret[file] = results[index];
});
externalLibs = removeDuplicates(result);
} catch (error) {
callback(error);
}

return async.parallel(
[
this._checkObjects.bind(this, results),
this._detectCycles.bind(this, results),
],
(checkingError) => {
if (checkingError) {
callback(checkingError);
}

console.log(`${'ok'.green} -- Template Parsing Finished`);
callback(null, ret);
},
);
},
let externalLibPaths = [];
if (externalLibs.length) {
externalLibPaths = externalLibs.map((lib) =>
recursiveReadDir(path.join(this.themePath, 'node_modules', lib, 'templates'), [
'!*.html',
]),
);
}
// eslint-disable-next-line node/no-unsupported-features/es-builtins
const res = await Promise.allSettled([
recursiveReadDir(this.templatesPath, ['!*.html']),
...externalLibPaths,
]);
const [{ value: internalTemplates }, ...externalTemplatesList] = res;
const internalPartials = internalTemplates.map((file) => {
return upath.toUnix(
file.replace(this.templatesPath + path.sep, '').replace(/\.html$/, ''),
);
});
const externalPartials = externalTemplatesList.reduce(
(partials, { value: externalTemplates }) => {
const extractedPartials = externalTemplates.map((file) => {
return upath.toUnix(
'external' + file.split('node_modules')[1].replace(/\.html$/, ''),
);
});
partials.push(...extractedPartials);

return partials;
},
[],
);
const allPartials = [...externalPartials, ...internalPartials];

let results;
try {
results = await async.map(
allPartials,
templateAssembler.assembleAndBundle.bind(null, this.templatesPath),
);

const ret = {};

allPartials.forEach((file, index) => {
ret[file] = results[index];
});

await Promise.all([
this._checkObjects.bind(this, results),
this._detectCycles.bind(this, results),
]);

console.log(`${'ok'.green} -- Template Parsing Finished`);

return ret;
Copy link
Contributor

Choose a reason for hiding this comment

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

did you try to use return callback(null, ret) here? What happened?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we received task object with field templates undefined which prevented us to create bundle

} catch (err) {
return callback(err);
}
}

async assembleSchema() {
Expand Down Expand Up @@ -317,56 +365,59 @@ class Bundle {
typeof this.options.dest === 'string' ? this.options.dest : this.themePath;
const bundleZipPath = path.join(outputFolder, outputName);

async.parallel(this.tasks, (err, taskResults) => {
if (err) {
return callback(err);
}
this.tasks.templates = this.assembleTemplatesTask.bind(this, callback);

const archive = Archiver('zip');
const fileStream = fs.createWriteStream(bundleZipPath);
archive.pipe(fileStream);

// Create manifest will use taskResults to generate a manifest file
return this.generateManifest(taskResults, (manifestGenerationError, manifest) => {
if (manifestGenerationError) {
return callback(manifestGenerationError);
}

// eslint-disable-next-line no-param-reassign
taskResults.manifest = manifest;
// zip theme files
this._bundleThemeFiles(archive, this.themePath);

// zip all generated files
const failedTemplates = this._bundleParsedFiles(archive, taskResults);

fileStream.on('close', () => {
const stats = fs.statSync(bundleZipPath);
const { size } = stats;

if (failedTemplates.length) {
return console.error(
`Error: Your bundle failed as templates generated from the files below are greater than or equal to 1 megabyte in size:\n${failedTemplates.join(
'\n',
)}`,
);
}
async
.parallel(this.tasks)
.then((taskResults) => {
const archive = Archiver('zip');
const fileStream = fs.createWriteStream(bundleZipPath);
archive.pipe(fileStream);

if (size > MAX_SIZE_BUNDLE) {
return console.error(
`Error: Your bundle of size ${size} bytes is above the max size of ${MAX_SIZE_BUNDLE} bytes`,
);
// Create manifest will use taskResults to generate a manifest file
return this.generateManifest(taskResults, (manifestGenerationError, manifest) => {
if (manifestGenerationError) {
return callback(manifestGenerationError);
}

console.log(`${'ok'.green} -- Zipping Files Finished`);
// eslint-disable-next-line no-param-reassign
taskResults.manifest = manifest;
// zip theme files
this._bundleThemeFiles(archive, this.themePath);

return callback(null, bundleZipPath);
});
// zip all generated files
const failedTemplates = this._bundleParsedFiles(archive, taskResults);

fileStream.on('close', () => {
const stats = fs.statSync(bundleZipPath);
const { size } = stats;

if (failedTemplates.length) {
return console.error(
`Error: Your bundle failed as templates generated from the files below are greater than or equal to 1 megabyte in size:\n${failedTemplates.join(
'\n',
)}`,
);
}

// This triggers 'close' event in the file stream. No need to callback()
return archive.finalize();
if (size > MAX_SIZE_BUNDLE) {
return console.error(
`Error: Your bundle of size ${size} bytes is above the max size of ${MAX_SIZE_BUNDLE} bytes`,
);
}

console.log(`${'ok'.green} -- Zipping Files Finished`);

return callback(null, bundleZipPath);
});

// This triggers 'close' event in the file stream. No need to callback()
return archive.finalize();
});
})
.catch((err) => {
console.log(err);
});
});
}

/**
Expand Down
18 changes: 10 additions & 8 deletions lib/stencil-bundle.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ describe('Stencil Bundle', () => {
});

it('should assembleTemplates', async () => {
const result = await promisify(bundle.assembleTemplatesTask.bind(bundle))();
const result = await bundle.assembleTemplatesTask.call(bundle);

expect(result['pages/page']).toEqual(
expect.objectContaining({
Expand All @@ -87,13 +87,15 @@ describe('Stencil Bundle', () => {
});

it('should error when running assembleTemplates', async () => {
jest.spyOn(async, 'map').mockImplementation((coll, iteratee, cb) =>
cb(new Error('our_error2')),
);
const errorMessage = 'our_error2';
const cb = (err) => {
if (err) throw err;
};
jest.spyOn(async, 'map').mockImplementation(() => {
throw new Error(errorMessage);
});

await expect(promisify(bundle.assembleTemplatesTask.bind(bundle))()).rejects.toThrow(
'our_error2',
);
await expect(bundle.assembleTemplatesTask.call(bundle, cb)).rejects.toThrow(errorMessage);
});

it('should assemble the Schema', async () => {
Expand Down Expand Up @@ -123,7 +125,7 @@ describe('Stencil Bundle', () => {
});

it('should generate a manifest of files.', async () => {
const templates = await promisify(bundle.assembleTemplatesTask.bind(bundle))();
const templates = await bundle.assembleTemplatesTask.call(bundle);
const manifest = await promisify(bundle.generateManifest.bind(bundle))({ templates });

expect(manifest.templates).toEqual(
Expand Down
6 changes: 6 additions & 0 deletions lib/template-assembler.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@ const dynamicComponentRegex = /\{\{\s*?dynamicComponent\s*(?:'|")([_|\-|a-zA-Z0-
const includeRegex = /{{2}>\s*([_|\-|a-zA-Z0-9/]+)[^{]*?}{2}/g;
const packageMarker = 'external/';
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe it's better to consolidate this name with externalTemplatesPath in stencil-bundle?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We removed externalTemplatesPath
If your comment is still valid please can you add more details:)


/**
* indicates if a template source is a node modules dependency
*
* @param {string} templateName
* @returns {Boolean}
*/
const isExternalTemplate = (templateName) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please add here and below doc types?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

return templateName.startsWith(packageMarker);
};
Expand Down