Mz/bundle core n templates #5705
Conversation
| @@ -96,11 +96,11 @@ | |||
| "dist" | |||
| ], | |||
| "packageUpdates": { | |||
| "main": "dist/index.js", | |||
| "main": "dist/src/index.js", | |||
| "salesforceApiVersion": "61", | |||
There was a problem hiding this comment.
I added this config in package.json of the core vsix, in order to provide the missing info from bundling template. So with this setting, one should be aware to update the field when the version of templates is bumped here with a bump of salesforceApiVersion.
I would also like to make this dynamic within esbuild.config.js, so one does not need to update the field frequently, but this added config should be always taken care.
The reason why it works is that the relative path of package.json of baseGenerator.ts within templates library is identical to the relative path of package.json within vsix (of dist/src/index.js).
There was a problem hiding this comment.
Not a big fan of this. Having this here does not guarantee it is correct. This data should come from templates module.
There was a problem hiding this comment.
I think "salesforceApiVersion" can be removed.
| @@ -6,7 +6,7 @@ | |||
| */ | |||
|
|
|||
| import { ConfigUtil } from '@salesforce/salesforcedx-utils-vscode'; | |||
| import { nls as templatesNls } from '@salesforce/templates-bundle/lib/i18n'; | |||
| import { nls as templatesNls } from '@salesforce/templates/lib/i18n'; | |||
There was a problem hiding this comment.
The usage like this should not work, but vscode-integration is deprecated now so we just ignore it.
| @@ -96,11 +96,11 @@ | |||
| "dist" | |||
| ], | |||
| "packageUpdates": { | |||
| "main": "dist/index.js", | |||
| "main": "dist/src/index.js", | |||
There was a problem hiding this comment.
changed the path of entry due to the reason from the other comments
| @@ -96,11 +96,11 @@ | |||
| "dist" | |||
| ], | |||
| "packageUpdates": { | |||
| "main": "dist/index.js", | |||
| "main": "dist/src/index.js", | |||
| "salesforceApiVersion": "61", | |||
There was a problem hiding this comment.
Not a big fan of this. Having this here does not guarantee it is correct. This data should come from templates module.
|
@peternhale I create a PR in templates to resolve the concern you raised. e2e test passes. |
|
|
||
| (async () => { | ||
| await build({ | ||
| ...sharedConfig, | ||
| entryPoints: ['./src/index.ts'], | ||
| outfile: 'dist/index.js' | ||
| outdir: 'dist/src' |
There was a problem hiding this comment.
@mingxuanzhangsfdx can this be simplified to 'dist'? That would then "match" the other extension bundles.
There was a problem hiding this comment.
I put the bundle artifacts inside a src folder in order to align the relative path to /templates with the original one. I can put them directly under dist, but that will make /templates parallel to /dist. So I need to adjust this during vscode:package process, which I found it hard to maintain. So the best solution is to construct it during /dist generation imho.
| @@ -96,11 +96,11 @@ | |||
| "dist" | |||
| ], | |||
| "packageUpdates": { | |||
| "main": "dist/index.js", | |||
| "main": "dist/src/index.js", | |||
| "salesforceApiVersion": "61", | |||
There was a problem hiding this comment.
I think "salesforceApiVersion" can be removed.
mingxuanzhangsfdx
requested review from
peternhale
and removed request for
lahernandezb
What does this PR do?
This PR further bundles sfdx-core and templates in core extension. The total size of core extension reduces from 8.9MB to 2.8MB
What issues does this PR fix or reference?
@W-15736969@
Functionality Before
sfdx-core and templates are not bundled in core extension.
Functionality After
sfdx-core and templates are bundled.