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

Jump to bottom

chore: make templates bundle-able #593

Merged
merged 12 commits into from
Aug 6, 2024
Merged

chore: make templates bundle-able #593

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
b5c8aa3
chore: transition to static imports for generators
mingxuanzhangsfdx Jul 31, 2024
a75c497
feat: make templates bundle-able
mingxuanzhangsfdx Aug 1, 2024
2728b42
chore: revert project name
mingxuanzhangsfdx Aug 1, 2024
cc61fcb
chore: refactor
mingxuanzhangsfdx Aug 1, 2024
54df39f
chore: revert esbuild-related artifacts
mingxuanzhangsfdx Aug 1, 2024
2159cc5
chore: revert esbuild-related artifacts
mingxuanzhangsfdx Aug 1, 2024
6346955
chore: revert esbuild-related artifacts
mingxuanzhangsfdx Aug 1, 2024
c2ed958
chore: remove bundle-related workflows
mingxuanzhangsfdx Aug 1, 2024
25b4956
chore: update map
mingxuanzhangsfdx Aug 6, 2024
0b7bda4
chore: suggestions
peternhale Aug 6, 2024
538b0a3
chore: apply suggestion
mingxuanzhangsfdx Aug 6, 2024
a9b2950
chore: apply suggestion
mingxuanzhangsfdx Aug 6, 2024
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
7 changes: 5 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@
"salesforceApiVersion": "61",
"description": "Salesforce JS library for templates",
"bugs": "https://github.com/forcedotcom/salesforcedx-templates/issues",
"main": "lib/index.js",
"main": "dist/src/index.js",
"author": "Salesforce",
"homepage": "https://github.com/forcedotcom/salesforcedx-templates",
"license": "BSD-3-Clause",
"files": [
"/lib",
"/dist",
peternhale marked this conversation as resolved.
Show resolved Hide resolved
"/messages"
],
"dependencies": {
Expand Down Expand Up @@ -38,6 +38,7 @@
"chai-as-promised": "^7.1.1",
"commitizen": "^4.3.0",
"cz-conventional-changelog": "^3.3.0",
"esbuild": "^0.23.0",
"eslint": "^7.27.0",
"eslint-config-prettier": "^6.11.0",
"eslint-config-salesforce": "^0.1.6",
Expand All @@ -49,6 +50,7 @@
"eslint-plugin-prettier": "^3.1.3",
"husky": "^8.0.3",
"mocha": "^10.4.0",
"npm-dts": "^1.3.12",
"nyc": "^15.1.0",
"prettier": "^2.0.5",
"pretty-quick": "^3.1.0",
Expand All @@ -68,6 +70,7 @@
"scripts": {
"build": "yarn run clean:lib && yarn build:templates && yarn compile",
"build:templates": "node scripts/build-templates",
"bundle": "yarn build && node scripts/esbuild.config.js",
"clean:lib": "shx rm -rf lib && shx rm -rf coverage && shx rm -rf .nyc_output",
"commit": "git-cz",
"compile": "tsc -b",
Expand Down
12 changes: 7 additions & 5 deletions scripts/build-templates.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@ const fs = require('fs');
const path = require('path');
const shell = require('shelljs');

// Determine the directory based on the 'esbuild' argument
const isEsbuild = process.argv.includes('--esbuild');
const currDir = process.cwd();
const libDir = path.join(currDir, 'lib');
const libTemplatesDir = path.join(libDir, 'templates');
const outputDir = isEsbuild ? 'dist' : 'lib';
const outputTemplatesDir = path.join(currDir, outputDir, 'templates');
const srcTemplatesDir = path.join(currDir, 'src', 'templates');

if (!fs.existsSync(libDir)) {
shell.mkdir(libDir);
if (!fs.existsSync(outputDir)) {
shell.mkdir(outputDir);
}

shell.cp('-rf', srcTemplatesDir, libTemplatesDir);
shell.cp('-rf', srcTemplatesDir, outputTemplatesDir);
41 changes: 41 additions & 0 deletions scripts/esbuild.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
const { build } = require('esbuild');
const { exec } = require('child_process');
const { Generator } = require('npm-dts');
new Generator({
output: `dist/src/index.d.ts`,
}).generate();

const sharedConfig = {
bundle: true,
format: 'cjs',
Copy link
Contributor

Choose a reason for hiding this comment

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

can the cli team consume a cjs module now that they are an esm shop? Might influence what we publish.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a good question. I will have a try.

Copy link
Member Author

Choose a reason for hiding this comment

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

Since we do not need to bundle the project here. I think we do not need to take care of the issue.

platform: 'node',
external: [],
minify: false,
keepNames: true,
plugins: [],
};

(async () => {
await build({
...sharedConfig,
entryPoints: ['./lib/index.js'],
outdir: 'dist/src',
});
})()
.then(() => {
exec(
'node ./scripts/build-templates.js --esbuild',
(error, stdout, stderr) => {
if (error) {
console.error(`Error executing command: ${error}`);
return;
}
if (stderr) {
console.error(`Error output: ${stderr}`);
return;
}
console.log(`Output: ${stdout}`);
}
);
})
.catch(() => process.exit(1));
53 changes: 45 additions & 8 deletions src/service/templateService.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,18 +5,55 @@
* For full license text, see LICENSE.txt file in the repo root or https://opensource.org/licenses/BSD-3-Clause
*/

import {
type CreateOutput,
type TemplateOptions,
TemplateType,
} from '../utils/types';
import { type CreateOutput, TemplateType } from '../utils/types';
peternhale marked this conversation as resolved.
Show resolved Hide resolved
import analyticsTemplateGenerator from '../generators/analyticsTemplateGenerator';
import apexClassGenerator from '../generators/apexClassGenerator';
import apexTriggerGenerator from '../generators/apexTriggerGenerator';
import lightningAppGenerator from '../generators/lightningAppGenerator';
import lightningComponentGenerator from '../generators/lightningComponentGenerator';
import lightningEventGenerator from '../generators/lightningEventGenerator';
import lightningInterfaceGenerator from '../generators/lightningInterfaceGenerator';
import lightningTestGenerator from '../generators/lightningTestGenerator';
import projectGenerator from '../generators/projectGenerator';
import staticResourceGenerator from '../generators/staticResourceGenerator';
import visualforceComponentGenerator from '../generators/visualforceComponentGenerator';
import visualforcePageGenerator from '../generators/visualforcePageGenerator';

type Generators =
| typeof analyticsTemplateGenerator
| typeof apexClassGenerator
| typeof apexTriggerGenerator
| typeof lightningAppGenerator
| typeof lightningComponentGenerator
| typeof lightningEventGenerator
| typeof lightningTestGenerator
| typeof lightningInterfaceGenerator
| typeof projectGenerator
| typeof staticResourceGenerator
| typeof visualforceComponentGenerator
| typeof visualforcePageGenerator;

const generators = new Map<string, Generators>([
['analyticsTemplateGenerator', analyticsTemplateGenerator],
['apexClassGenerator', apexClassGenerator],
['apexTriggerGenerator', apexTriggerGenerator],
['lightningAppGenerator', lightningAppGenerator],
['lightningComponentGenerator', lightningComponentGenerator],
['lightningEventGenerator', lightningEventGenerator],
['lightningInterfaceGenerator', lightningInterfaceGenerator],
['lightningTestGenerator', lightningTestGenerator],
['projectGenerator', projectGenerator],
['staticResourceGenerator', staticResourceGenerator],
['visualforceComponentGenerator', visualforceComponentGenerator],
['visualforcePageGenerator', visualforcePageGenerator],
]);

export async function importGenerator(templateType: TemplateType) {
peternhale marked this conversation as resolved.
Show resolved Hide resolved
const generatorClass =
TemplateType[templateType].toString().charAt(0).toLowerCase() +
TemplateType[templateType].toString().slice(1) +
'Generator';
return (await import(`../generators/${generatorClass}`)).default;
return generators.get(generatorClass) as Generators;
Copy link
Contributor

Choose a reason for hiding this comment

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

The as is masking the fact that generators.get(...) can return undefined. This flaw exists in the original code and assumes that the generator author has everything lined up properly. However, the original code would fail during the dynamic import if the calculated path to the template is incorrect.

How can the dev side of adding a template be made more concise and safe with this change in how we need to expose the generators for bundling?

}

/**
Expand Down Expand Up @@ -64,9 +101,9 @@ export class TemplateService {
* @param templateOptions template options
* @param customTemplatesRootPathOrGitRepo custom templates root path or git repo. If not specified, use built-in templates
*/
public async create<TOptions extends TemplateOptions>(
public async create(
templateType: TemplateType,
templateOptions: TOptions,
templateOptions: any,
Copy link
Contributor

Choose a reason for hiding this comment

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

nice

Copy link
Contributor

Choose a reason for hiding this comment

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

Why has this type changed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Argument of type 'TOptions' is not assignable to parameter of type 'never'.
  The intersection 'AnalyticsTemplateOptions & ApexClassOptions & ApexTriggerOptions & LightningAppOptions & ... 7 more ... & VisualforcePageOptions' was reduced to 'never' because property 'template' has conflicting types in some constituents.
    Type 'TemplateOptions' is not assignable to type 'never'.ts(2345)

I ran into the issue above if I keep TOptions. I found that it will be much easier to just use any

customTemplatesRootPathOrGitRepo?: string
): Promise<CreateOutput> {
const runOptions = {
Expand Down
Loading
Loading