Adds Heft tasks. Closes #618

[Adam-it]

🏗️ 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 heft tasks 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

• [X] Refactored the icons and naming a bit
• [X] Added heft tasks to the task view in case the project uses heft npm package
• [ ] Update Task View tests to support heft
• [ ] Optimize code, currently most is duplicated but it seems many methods may be reused and we only need to pass if gulp or heft should be used in the command
• [ ] It seems heft does not support start (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

[Adam-it]

@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. In the past (SPFx 1.21 and earlier) we needed to have gulp-cli installed as one of the global dependencies and Gulp locally (in project) is because of the incompatibility of Gulp v3 & v4. The Gulp CLI, installed globally, could abstract its incompatibility so you could use gulp at the command line anywhere (https://www.voitanos.io/blog/mea-culpa-always-install-gulp-cli-globally-not-gulp/)

But now it is not the case anymore. For SPFx 1.22 which now will use heft we do not need heft installed globally anymore and we may just use the one which is part of the SPFx project installed locally using ./node_modules/.bin/heft ... like ./node_modules/.bin/heft build.

To prove this I was managed to build a SPFx 1.22 project using npm run build which runs heft build --clean withouth having heft installed locally. When I just run heft build it fails as I do not have the package

SO, I believe this PR may actually bring the following problems:

• heft tasks we expose as part of this PR will only work if you have heft installed globally which is not needed
• the heft tasks mostly duplicate the once exposed as part of npm run command so it clutters the view. If we just leave npm tree for the view will be smaller and still have everything that is needed for the SPFx project

for older projects we still need to show both npm tree (which is kinda useless for older projects. and gulp which expose gulp-cli tasks. So in this case we should not change anything.

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

[Adam-it]

So I did some more testing of SPFx 1.22 and I think that heft is indeed needed. For example, one problem is that with just npm run build or package-solution you may not build the package for production, you still need heft to do that.

So now I think this PR might be worth adding and also installing heft globally as well in the install action. @pnp/spfx-toolkit-maintainers what do you think?

[Saurabh7019]

I think this PR should be added after some modifications..

installing heft globally as well in the install action.

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 npx

the heft tasks mostly duplicate the once exposed as part of npm run command so it clutters the view.

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 package.json scripts. Projects work fine even if the scripts are removed. Whereas, NPM scripts allow us to create custom scripts and access them easily via our Tasks view with one click.

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:

• Keep everything as-is for the older versions and then add support for new toolchain.
• Add a utility function to check if a project is heft-based
• Update the existing SPFx tasks functions to run the gulp/heft commands
• Use local project's heft installation using npx
• Collapse 'NPM Scripts' node by default and also consider filtering out the core SPFX tasks

[Adam-it]

@Saurabh7019 thanks for the comment. to tackle some of the things you suggested

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 npx

thats true. In the docs currently it is only specified as a TIP

In this case I think using npx is a way better option. Good idea 👍👍👍

So, what I am proposing is:

• Keep everything as-is for the older versions and then add support for new toolchain.

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

• Add a utility function to check if a project is heft-based

Yes and this PR already does that. We have a shouldShowHeftTasks method that checks if npm package @rushstack/heft is present in the package.json. I created a similar to for gulp that checks if gulpfile.js is present in the project. Theortically it is possible to have all three: gulp + heft + npm scripts, and it will work, although I think no one should have such a setting

• Update the existing SPFx tasks functions to run the gulp/heft commands

yes. I have this as part of tasks to finish in this PR. In the PR descirption I left it as an open point

- [ ] Optimize code, currently most is duplicated but it seems many methods may be reused and we only need to pass if gulp or heft should be used in the command

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 heft start command. This was a critical error I hope it was addressed. Still need to recheck it with RC release 🤔

• Use local project's heft installation using npx

This is a great idea! One question: what if someone has heft installed globally anyway? does the npx cover this case under the hood and uses the global installed or still uses the temp download one 🤔? or maybe we should handle it and check if someone has global heft we use this one, and if not then we use npx 🤔?

• Collapse 'NPM Scripts' node by default and also consider filtering out the core SPFX tasks

If we collapse maybe both should be collapsed 🤔? as for the filtering out the core SPFx tasks is a 'touch' subject.. what do you mean by 'core'? for some its just build and package-soltion,. Does the serve/start count 🤔? I am not a huge fan of this feed, as it may seem opinionated 🙂

[Saurabh7019]

The PR which was merged a few weeks ago already introduced a tree view in tasks view having both gulp and npm scripts

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.

• cleanProject
• buildProject
• testProject
• trustDevCert
• deployToAzureStorage
• serveProject
• bundleProject
• packageProject
• publishProject
• cleanProject

[Saurabh7019]

One question: what if someone has heft installed globally anyway?

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.

[Saurabh7019]

what do you mean by 'core'? for some its just build and package-soltion,.

for me, it is clean, bundle, package, serve, test but I agree, filtering out what we think are the core tasks might not be a good idea. We should just keep the nodes collapsed.

[Adam-it]

The PR which was merged a few weeks ago already introduced a tree view in tasks view having both gulp and npm scripts

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.

• cleanProject
• buildProject
• testProject
• trustDevCert
• deployToAzureStorage
• serveProject
• bundleProject
• packageProject
• publishProject
• cleanProject

But is it optimal? Why not rename them to be more heft related? There is no serve in heft bit start, also there is no bundle

[Saurabh7019]

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.

[Adam-it]

@pnp/spfx-toolkit-maintainers, this PR is ready for review and merge to go with the upcoming minor for the SPFx 1.22 release. I struggled a bit to find time on this and did not fully cover it with new tests. We could either add it as a follow-up PR or if one of you got any time to help me out, it would be great. Along the way, I corrected a few things in the documentation:

• broken links
• We forgot to update the upgrade project action documentation so I fixed that as well

@Saurabh7019, since you already have some time to give this PR a check, maybe you would like to continue on this 😉

[Saurabh7019]

Awesome work, @Adam-it ! This is merged now.