spfx project upgrade fails when .yo-rc.json & package.json are out of sync

[pschaeflein]

Description

The spfx project upgrade command returned a message that project doesn't need to be upgraded despite the package.json references to old versions of @microsoft/sp* packages

Steps to reproduce

In an spfx project, the .yo-rc.json file contained current spfx version:

{
  "@microsoft/generator-sharepoint": {
    "isCreatingSolution": true,
    "environment": "spo",
    "version": "1.10.0",
    "libraryName": "library",
    "libraryId": "db85a7f3***",
    "packageManager": "npm",
    "componentType": "extension",
    "extensionType": "ApplicationCustomizer"
  }
}

The package.json file contained references to older versions

{
       "dependencies": {
                "@microsoft/sp-application-base": "1.9.1",
                "@microsoft/sp-core-library": "1.9.1",
                "@microsoft/sp-dialog": "1.9.1",
    }
}

Expected result

I expected a report of changes to make to upgrade the project.

Actual result

"Error: Project doesn't need to be upgraded"

Environment

Win10, PWSH
o365 version: 2.8.0-beta.ee3c531

[waldekmastykarz]

Good catch @pschaeflein. I'd suggest we change detecting the project version to use the package version instead of .yo-rc.json which is more reliable. Agreed @VelinGeorgiev @garrytrinder?

[garrytrinder]

Agreed, package.json is the source of truth here so we should check versions here first.

It maybe worth considering returning a message in the upgrade output to state that the .yo-rc.json SPFX version does not match the installed dependencies.

[VelinGeorgiev]

I am ok with this.

[waldekmastykarz]

So this seems to be harder than it seems to do correctly.

Some versions of SPFx use different versions of the core packages, eg.:

1.3.2

"@microsoft/sp-core-library": "~1.3.0"
"@microsoft/sp-webpart-base": "~1.3.0"

1.1.3:

"@microsoft/sp-core-library": "~1.1.0"
"@microsoft/sp-webpart-base": "~1.1.1"

1.1.1:

"@microsoft/sp-webpart-base": "~1.1.1"
"@microsoft/sp-core-library": "~1.1.0"
"@microsoft/sp-build-web": "~1.1.0"

1.0.2

"@microsoft/sp-core-library": "~1.0.0"
"@microsoft/sp-webpart-base": "~1.0.0"
"@microsoft/sp-build-web": "~1.0.1"

1.0.1

"@microsoft/sp-build-web": "~1.0.1"
"@microsoft/sp-core-library": "~1.0.0"
"@microsoft/sp-webpart-base": "~1.0.0"

Right now, we use the @microsoft/sp-core-library package to determine the version but based on the two examples above, it would return wrong version in multiple cases.

Event by looking at multiple packages and adjusting the order to something like this:

@microsoft/sp-application-base
@microsoft/sp-webpart-base
@microsoft/sp-build-web
@microsoft/sp-core-library

It would still return incorrect version v1.0.1 where sp-webpart-base is v1.0.0 as well as some other version that don't have corresponding package versions.

With such inconsistencies in place, I'd say that we keep what we have because it seems like the most reliable way of detecting project version.
We could extend the upgrade process with a check if .yo-rc.json and package.json are in sync, but again, for some versions it would lead to an error/message.

What do you think @pschaeflein @garrytrinder @VelinGeorgiev?

[garrytrinder]

Great analysis @waldekmastykarz 👏🏻

I agree that we should keep the current check in place based on your findings.

[pschaeflein]

If I run the job today, do I care if the project is 1.1.1 or 1.1.3? I mean, it ends up as 1.10 afterwards, right? Not sure if these old, incremental differences matter.

[waldekmastykarz]

They actually might. The way upgrade guidance is done is that v1.8 only knows how to upgrade v1.7 project. It doesn't know what's been done in the previous versions. So theoretically, if there is anything relevant done between v1.1.1 and v1.1.3 the upgrade guidance wouldn't contain these changes and you could end up with a broken project. That's theory. I've never looked at what's the reality of this because the mechanism worked so far, until you get to an issue like you mentioned :)

How about an alternative: before we start the upgrade logic, we add extra logic to verify the project's integrity and if there is anything out of the ordinary, we communicate that first. Would that address the issue you mentioned @pschaeflein?

[pschaeflein]

I have only upgraded one project in the years that SPFx/CLI existed, so not sure I’m the best to source here. I would simply say there is little need for a volunteer project to invest time in edge cases. How edge case is defined, I will leave to others. ;)

[waldekmastykarz]

@garrytrinder @VelinGeorgiev any feedback from your side?

[garrytrinder]

before we start the upgrade logic, we add extra logic to verify the project's integrity and if there is anything out of the ordinary, we communicate that first.

I would say that anything we can add to warn the user of a potential issue then we should. Good suggestion 👍🏻

[waldekmastykarz]

All right. I'll create an issue to extend the spfx upgrade command with an extra integrity check upfront.

[waldekmastykarz]

On a different note @pschaeflein, do you know how your project ended up in this state (meaning different version in .yo-rc.json and package.json)?

[pschaeflein]

I don't know for sure, but my hunch is that a previous upgrade omitted that step.