-
Notifications
You must be signed in to change notification settings - Fork 26
Adds Heft tasks. Closes #618 #626
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
base: dev
Are you sure you want to change the base?
Conversation
|
@pnp/spfx-toolkit-maintainers I am wondering if this PR and functionality is actually needed. I am afraid it will create more confusion and I will try to explain why. But now it is not the case anymore. For SPFx 1.22 which now will use heft we do not need To prove this I was managed to build a SPFx 1.22 project using SO, I believe this PR may actually bring the following problems:
for older projects we still need to show both What do you think? After more tests of 1.22 and investigating around I suggest NOT to merge this PR and leave only npm scripts tree in our task view for project that are SPFx 1.22 and newer versions |
|
So I did some more testing of SPFx 1.22 and I think that So now I think this PR might be worth adding and also installing heft globally as well in the install action. |
|
I think this PR should be added after some modifications..
this might not be a good idea. If SPFx 1.22 docs doesn't specifically recommend installing heft globally then we shouldn't do it. Installing it globally using SPFx Toolkit could raise questions from the community. Instead, we can use local project's heft installation using
This duplication issue is there for older SPFx versions too (Gulp Tasks vs NPM Scripts), but I believe showing both is required for different reasons.. SPFx tasks are core functionality and they are essential operations that work irrespective of what's in to reduce the clutter, we can hide NPM scripts that matches our core SPFx tasks (clean, build, bundle, etc.). We should also collapse the NPM Scripts node by default to start with, If it is tricky and time taking to implement this filtering. So, what I am proposing is:
|
| private static gulpBuildProject() { | ||
| commands.executeCommand(Commands.executeTerminalCommand, 'gulp build'); | ||
| } |
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.
| private static gulpBuildProject() { | |
| commands.executeCommand(Commands.executeTerminalCommand, 'gulp build'); | |
| } | |
| private static buildProject() { | |
| // if heftProject | |
| commands.executeCommand(Commands.executeTerminalCommand, 'npx heft build --clean'); | |
| // else | |
| commands.executeCommand(Commands.executeTerminalCommand, 'gulp build'); | |
| } |
something like this?
|
@Saurabh7019 thanks for the comment. to tackle some of the things you suggested
thats true. In the docs currently it is only specified as a TIP
In this case I think using
IMO this is already to late for that. The PR which was merged a few weeks ago already introduced a tree view in tasks view having both gulp and npm scripts
Yes and this PR already does that. We have a
yes. I have this as part of tasks to finish in this PR. In the PR descirption I left it as an open point The current state of the PR is like POC/work in progress. I had to stop it at this stage as for the SPFx 1.22 beta 4 (this was the latest SPFx 1.22 I used to develop this PR) did not support picking serve configurations for the
This is a great idea! One question: what if someone has
If we collapse maybe both should be collapsed 🤔? |
This is exactly what we want.. keep both Gulp Tasks (rename it to SPFx Tasks) and NPM Scripts, means no UI changes. For older versions, everything will continue to work as before. For newer versions, we can update the functions below to run the Heft tasks as well.
|
I am not sure.. I would assume if someone has heft installed globally, npx heft will use the global version. we can test this and add the check if needed as you suggested. |
for me, it is clean, bundle, package, serve, test |
But is it optimal? Why not rename them to be more heft related? |
hmm I wanted to avoid touching the UI layer and was also thinking about further (what if?) toolchain changes but I think you are right. having separate functions makes handling version specific tasks much easier. |


🏗️ Work in progress
This is still work in progress and we may not merge it just yet as this was developed based on SPFx 1.22-beta.4 so still some things may change before the official GA, but this PR should give us a head start when it comes to adding heft tasks support to this extension
🎯 Aim
The aim is to refactor the task view to show
hefttasks in case the SPFx project uses primary heft for build toolchain tasks📷 Result
SPFx 1.22 with Heft
SPFx 1.21 with Gulp
✅ What was done
gulporheftshould be used in the commandheftdoes not supportstart(previously serve) with a specific configuration, like for command sets. Left a TODO for it in code and checking this with SPFx team🔗 Related issue
Closes: #618