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

πŸ’‘[Feature]: Create one task for bundle & package each (Fix - 386) #397

Draft
wants to merge 3 commits into
base: dev
Choose a base branch
from
Draft
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
18 changes: 17 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,23 @@ Sign-in is also required for some actions to work properly like the deploy actio

The extension shows all possible Gulp tasks one may run on an SPFx project. The tasks allow you to clean, bundle, package, serve the project with a single click.

![Gulp Tasks](./assets/images/tasks.png)
![Gulp Tasks](./assets/images/tasks2.png)

- **gulp bundle**

![Bundle local](./assets/images/bundle-local.gif)

- **gulp bundle --ship**

![Bundle `production`](./assets/images/bundle-production.gif)

- **gulp package-solution**

![Package local](./assets/images/package-local.gif)

- **gulp package-solution --ship**

![Bundle `production`](./assets/images/package-production.gif)
Comment on lines +176 to +192
Copy link
Member

Choose a reason for hiding this comment

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

I appreciate taking your time to include and improve it in the readme as well but I would try to make it shorter. The readme is a first glance overall view of the product before you install it to VS Code so for gulp tasks I would just leave what we have and instead add the detail description and the gifs you provided to our wiki. At the end of each section, also here in gulp tasks, user has a link to wiki page for more details regarding that part

Suggested change
![Gulp Tasks](./assets/images/tasks2.png)
- **gulp bundle**
![Bundle local](./assets/images/bundle-local.gif)
- **gulp bundle --ship**
![Bundle `production`](./assets/images/bundle-production.gif)
- **gulp package-solution**
![Package local](./assets/images/package-local.gif)
- **gulp package-solution --ship**
![Bundle `production`](./assets/images/package-production.gif)
![Gulp Tasks](./assets/images/tasks2.png)


[Check out our docs for more details](https://github.com/pnp/vscode-viva/wiki/5.4-Gulp-tasks)

Expand Down
Binary file added assets/images/bundle-local.gif
Copy link
Member

Choose a reason for hiding this comment

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

Let's remove this file from the assets folder as if we follow my previous suggestion to no include it in the readme it will be not needed

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added assets/images/bundle-production.gif
Copy link
Member

Choose a reason for hiding this comment

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

Let's remove this file from the assets folder as if we follow my previous suggestion to no include it in the readme it will be not needed

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added assets/images/package-local.gif
Copy link
Member

Choose a reason for hiding this comment

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

Let's remove this file from the assets folder as if we follow my previous suggestion to no include it in the readme it will be not needed

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added assets/images/package-production.gif
Copy link
Member

Choose a reason for hiding this comment

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

Let's remove this file from the assets folder as if we follow my previous suggestion to no include it in the readme it will be not needed

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added assets/images/tasks2.png
Copy link
Member

Choose a reason for hiding this comment

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

Instead of adding an additional image lets replace the current one. Also, each image should follow similar theming to be consistent and in this case it should be the default VS Code light theme

image

Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 1 addition & 1 deletion assets/walkthrough/tasks.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,6 @@ deploy-azure-storage - Deploys client-side solution project assets to Azure Stor

SPFx Toolkit VS Code extension shows all possible Gulp tasks one may run on an SPFx project. Don't worry about remembering all the commands, just click on the task you want to run and the extension will do the rest.

![Gulp Tasks](../images/tasks.png)
![Gulp Tasks](../images/tasks2.png)
Copy link
Member

Choose a reason for hiding this comment

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

if we replace the old image instead of adding a new one then lets ever this change as well


[Check out our docs for more details](https://github.com/pnp/vscode-viva/wiki/5.4-Gulp-tasks)
6 changes: 6 additions & 0 deletions src/constants/Commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,12 @@ export const Commands = {
// Webviews
samplesGallery: `${EXTENSION_NAME}.samplesGallery`,

// Bundle
bundleProject: `${EXTENSION_NAME}.bundleProject`,

// Package
packageProject: `${EXTENSION_NAME}.packageProject`,

// Serving
serveProject: `${EXTENSION_NAME}.serveProject`,

Expand Down
6 changes: 2 additions & 4 deletions src/panels/CommandPanel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -285,12 +285,10 @@ export class CommandPanel {
private static taskTreeView() {
const taskCommands: ActionTreeItem[] = [
new ActionTreeItem('Build project', '', { name: 'debug-start', custom: false }, undefined, Commands.executeTerminalCommand, 'gulp build'),
new ActionTreeItem('Bundle project (local)', '', { name: 'debug-start', custom: false }, undefined, Commands.executeTerminalCommand, 'gulp bundle'),
new ActionTreeItem('Bundle project (production)', '', { name: 'debug-start', custom: false }, undefined, Commands.executeTerminalCommand, 'gulp bundle --ship'),
new ActionTreeItem('Bundle project', '', { name: 'debug-start', custom: false }, undefined, Commands.bundleProject),
new ActionTreeItem('Clean project', '', { name: 'debug-start', custom: false }, undefined, Commands.executeTerminalCommand, 'gulp clean'),
new ActionTreeItem('Deploy project assets to Azure Storage', '', { name: 'debug-start', custom: false }, undefined, Commands.executeTerminalCommand, 'gulp deploy-azure-storage'),
new ActionTreeItem('Package (local)', '', { name: 'debug-start', custom: false }, undefined, Commands.executeTerminalCommand, 'gulp package-solution'),
new ActionTreeItem('Package (production)', '', { name: 'debug-start', custom: false }, undefined, Commands.executeTerminalCommand, 'gulp package-solution --ship'),
new ActionTreeItem('Package', '', { name: 'debug-start', custom: false }, undefined, Commands.packageProject),
new ActionTreeItem('Serve', '', { name: 'debug-start', custom: false }, undefined, Commands.executeTerminalCommand, 'gulp serve'),
new ActionTreeItem('Serve (nobrowser)', '', { name: 'debug-start', custom: false }, undefined, Commands.executeTerminalCommand, 'gulp serve --nobrowser'),
new ActionTreeItem('Serve from configuration', '', { name: 'debug-start', custom: false }, undefined, Commands.serveProject),
Expand Down
34 changes: 34 additions & 0 deletions src/services/executeWrappers/TerminalCommandExecuter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ export class TerminalCommandExecuter {
subscriptions.push(
commands.registerCommand(Commands.serveProject, TerminalCommandExecuter.serveProject)
);
subscriptions.push(
commands.registerCommand(Commands.bundleProject, TerminalCommandExecuter.bundleProject)
);
subscriptions.push(
commands.registerCommand(Commands.packageProject, TerminalCommandExecuter.packageProject)
);
subscriptions.push(
commands.registerCommand(Commands.executeTerminalCommand, TerminalCommandExecuter.runCommand)
);
Expand Down Expand Up @@ -93,6 +99,34 @@ export class TerminalCommandExecuter {
commands.executeCommand(Commands.executeTerminalCommand, `gulp serve --config=${answer}`);
}

public static async bundleProject() {
const tasks = ['local','production'];
const answer = await window.showQuickPick(tasks, {
title: 'Select the target environment',
ignoreFocusOut: true
});

if(!answer) {
return;
}
Comment on lines +103 to +111
Copy link
Member

Choose a reason for hiding this comment

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

This part is actually duplicated in both methods. Let's extract it to a small helper private method in the same class and reuse instead


commands.executeCommand(Commands.executeTerminalCommand, answer === 'local' ? 'gulp bundle' : 'gulp bundle --ship');
Copy link
Member

Choose a reason for hiding this comment

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

We could simplify and make it a bit shorter like.

Suggested change
commands.executeCommand(Commands.executeTerminalCommand, answer === 'local' ? 'gulp bundle' : 'gulp bundle --ship');
commands.executeCommand(Commands.executeTerminalCommand, `gulp bundle${answer === 'local' ? '' : ' --ship'}`);

What do you think?
We could do the same for package as well

}

public static async packageProject() {
const tasks = ['local','production'];
const answer = await window.showQuickPick(tasks, {
title: 'Select the target environment',
ignoreFocusOut: true
});

if(!answer) {
return;
}

commands.executeCommand(Commands.executeTerminalCommand, answer === 'local' ? 'gulp package-solution' : 'gulp package-solution --ship');
}

/**
* Initializes the shell path for executing terminal commands.
* If the shell path is an object with a `path` property, it sets the `shellPath` to that value.
Expand Down