-
Notifications
You must be signed in to change notification settings - Fork 393
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: update bundling for core extension to exclude functions-core #4532
Conversation
import { | ||
CancelResponse, | ||
ContinueResponse, | ||
FunctionInfo, | ||
ParametersGatherer | ||
FunctionInfo, LibraryCommandletExecutor, ParametersGatherer | ||
} from '@salesforce/salesforcedx-utils-vscode'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just ran Organize imports b/c I was here and it needed it.
@@ -25,7 +25,7 @@ | |||
"Other" | |||
], | |||
"dependencies": { | |||
"@heroku/functions-core": "^0.4.0", | |||
"@heroku/functions-core": "0.4.2", | |||
"@octokit/plugin-paginate-rest": "2.21.3", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So on review of this flow I realized it's probably not a good thing to have ^ deps in the dependancies list and not when we do the packaging so I just pinned both to 4.0.2 which was the current version.
@@ -86,6 +86,7 @@ | |||
"main": "dist/index.js", | |||
"dependencies": { | |||
"applicationinsights": "1.0.7", | |||
"@heroku/functions-core": "0.4.2", | |||
"@salesforce/core": "3.30.9", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add functions-core as a module to be installed for the packaging. The size increase was pretty minimal I think, but will verify.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious what the difference was here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
previously the code was functions core was included in our 'bundle'
This was the root of the bug as they depend on being able to read a templates directory in their package node module and it didn't exist after bundling. I can show you if this makes no sense.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also I opened a bug in that repo about them not being 'bundle-able'. We'll see if it can pick up some traction
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Before:
Uploading /home/circleci/project/extensions/salesforcedx-vscode-core-56.4.0.vsix (24 MB): DONE
After:
Uploading /home/circleci/project/extensions/salesforcedx-vscode-core-56.4.0.vsix (29 MB): DONE
5 MB diff or a 20% size increase 😢 Not as trivial as I'd hoped
@@ -95,7 +96,7 @@ | |||
} | |||
}, | |||
"scripts": { | |||
"bundle:extension": "esbuild ./src/index.ts --bundle --outfile=dist/index.js --format=cjs --platform=node --external:vscode --external:@salesforce/core --external:applicationinsights --external:shelljs --external:@salesforce/templates --external:@salesforce/source-deploy-retrieve --minify", | |||
"bundle:extension": "esbuild ./src/index.ts --bundle --outfile=dist/index.js --format=cjs --platform=node --external:vscode --external:@salesforce/core --external:applicationinsights --external:shelljs --external:@salesforce/templates --external:@salesforce/source-deploy-retrieve --external:@heroku/functions-core --minify", | |||
"vscode:prepublish": "npm prune --production", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update the bundle command to not bundle functions-core
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
) * fix: update bundling for core extension to exclude functions-core fix #4531 * chore: bump npm version running package job * chore: try different npm install route
What does this PR do?
Update the core extension bundling to exclude the functions-core module from bundling.
Testing
To test this change do the following:
This will generate the
salesforcedx-vscode-core-56.4.0.vsix
extension.To install:
Go to whatever version of vscode you'd like to use for verifying the extension. Uninstall add salesforce extension & reload.
Click the '...' button at the top of the extensions view and then 'install from vsix'
select the generated vsix file
The bundled extension should now be installed in your vscode and you can test with
'SFDX: Create Function'
What issues does this PR fix or reference?
fix #4531
@W-11998949@
Functionality Before
Create Functions command fails for all types when used from a bundled core extension.
Functionality After
Create Function works using bundled core extension.