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

dataVersion defined in manifest is not respected when adding a WebPart in a page #8199

Closed
3 of 9 tasks
mgrosperrin opened this issue Jun 15, 2022 · 27 comments
Closed
3 of 9 tasks
Labels
area:spfx Category: SharePoint Framework (not extensions related) status:fixed-next-drop Issue planned to be fixed in an upcoming release. type:bug-suspected Suspected bug (not working as designed/expected). See “type:bug-confirmed” for confirmed bugs.
Milestone

Comments

@mgrosperrin
Copy link

Target SharePoint environment

SharePoint Online

What SharePoint development model, framework, SDK or API is this about?

💥 SharePoint Framework

Developer environment

Windows

What browser(s) / client(s) have you tested

  • 💥 Internet Explorer
  • 💥 Microsoft Edge
  • 💥 Google Chrome
  • 💥 FireFox
  • 💥 Safari
  • mobile (iOS/iPadOS)
  • mobile (Android)
  • not applicable
  • other (enter in the "Additional environment details" area below)

Additional environment details

  • browser version : latest at this time (Edge/Chrome: 102, Firefox:101)
  • SPFx version: 1.14
  • Node.js version: 12.13.1
  • etc

Describe the bug / error

I have created a WebPart and I need to change the model of the data associated with it.
Following the documentation, I bump dataVersion to 2.0 in the manisfest (preconfiguredEntries.dataVersion) and in the property of the WebPart, and implement the method onAfterDeserialize to handle the migration of the data from the old model to the new one. I add a check to only perform this migration if the version of the data being deserialized is 1.0.

When I run this code "locally" (with gulp serve), everything is working as expected: the onAfterDeserialize have a data version of 2.0 when I add the WebPart to a page.
But when I deploy the package containing the WebPart to the app catalog, the method onAfterDeserialize receives the value 1.0 for the parameter dataVersion.

Steps to reproduce

  1. Create a package with a WebPart
  2. In the manifest of the WebPart, change the value of the property preconfiguredEntries.dataVersion to 2.0
  3. In the WebPart, save & display the value of the parameter dataVersion of the method onAfterDeserialize
  4. In the WebPart implement the dataVersion property to return 2.0
  5. Deploy the package to an AppCatalog
  6. Add the WebPart to a page => the displayed value will be 1.0
  7. Save the page, and after the reload the value will be 2.0 (as expected)

Expected behavior

In the step 6, the newly added WebPart should display 2.0.

@mgrosperrin mgrosperrin added the type:bug-suspected Suspected bug (not working as designed/expected). See “type:bug-confirmed” for confirmed bugs. label Jun 15, 2022
@ghost
Copy link

ghost commented Jun 15, 2022

Thank you for reporting this issue. We will be triaging your incoming issue as soon as possible.

@ghost ghost added the Needs: Triage 🔍 Awaiting categorization and initial review. label Jun 15, 2022
@AJIXuMuK
Copy link
Collaborator

Thank you @mgrosperrin for reporting this one.

I can repro this issue and we'll be working on the fix.

One remark: preconfiguredEntries.dataVersion is not a valid manifest property. So no need to set it.
Screen Shot 2022-06-15 at 10 54 31 AM

@AJIXuMuK AJIXuMuK added area:spfx Category: SharePoint Framework (not extensions related) status:working-on-it Known issue / feature being addressed. Will use other "status:*" labels & comments for more detail. and removed Needs: Triage 🔍 Awaiting categorization and initial review. labels Jun 15, 2022
@mgrosperrin
Copy link
Author

mgrosperrin commented Jun 15, 2022

Hi @AJIXuMuK,
Thanks for your quick reply.

Regarding the preconfiguredEntries.dataVersion property, based on this documentation https://docs.microsoft.com/en-us/sharepoint/dev/spfx/web-parts/guidance/simplify-adding-web-parts-with-preconfigured-entries#web-part-preconfigured-entries, this should be a valid entry in the manifest.

I will open another issue for that 😄 : #8204

@AJIXuMuK
Copy link
Collaborator

@mgrosperrin - the problem is in docs, not in the manifest's schema

@mgrosperrin
Copy link
Author

@AJIXuMuK I don't really understand your answer.
If this field is not valid in the manifest, how can we specify to which version correspond the preconfigured properties defined in the manifest?

@mgrosperrin
Copy link
Author

To clarify my lack of understanding, I refer to this issue #1522.
@mpasarin say in this comment #1522 (comment), that the dataVersion is a runtime component because we can update the code without updating the manifest (which is correct). But the preconfigured entries are in the manifest, so we can't rely on a runtime property to know to which version the values in the manifest refers to.

Thanks for your clarification on this 😄

@AJIXuMuK
Copy link
Collaborator

@mgrosperrin - there are multiple issues related to this topic.
In few words, initially there was a manifest property dataVersion that has been removed later.

But it works incorrectly for sure for the adding web part scenario. And we're working on the fix.

@AJIXuMuK AJIXuMuK added status:fixed-next-drop Issue planned to be fixed in an upcoming release. and removed status:working-on-it Known issue / feature being addressed. Will use other "status:*" labels & comments for more detail. labels Jun 20, 2022
@AJIXuMuK AJIXuMuK modified the milestone: 06-17 Jun 20, 2022
@mgrosperrin
Copy link
Author

Hi @AJIXuMuK , do you have any news to share about this issue and the fix you are working on?

@AJIXuMuK
Copy link
Collaborator

AJIXuMuK commented Jul 6, 2022

Hi @mgrosperrin - sure!
It has been fixed and will be rolled out in the next couple of weeks.

@AJIXuMuK AJIXuMuK added this to the 07-01 milestone Jul 6, 2022
@mgrosperrin
Copy link
Author

@AJIXuMuK Happy to hear that!
Can you explain me what will be the new behavior to deal with the evolution of the preconfigured entries in the manifest?

@AJIXuMuK
Copy link
Collaborator

AJIXuMuK commented Jul 6, 2022

@mgrosperrin - if you add a new web part to the page, you will have dataVersion set to the current value from the property.
If you deal with existing web part (previously added) the dataVersion will be the version of the web part when it was last time saved.
preconfiguredEntries are taken into consideration when you add the web part to the page. In other situations you should "migrated" deserialized properties to meet new dataVersion.

@mgrosperrin
Copy link
Author

@AJIXuMuK thanks for the information.
The missing scenario (the one for which I create the issue 😄 ) is:
I create a WebPart with dataVersion set to 1.0 (the property) and some data from the preconfiguredEntries.
Later I have to update the schema of the data so I bump the dataVersion property to 2.0, implement the onAfterDeserialize method to handle the logic of updating the schema of the data (based on the provided version). When I add the WebPart to the page, I will have the "old" schema's data, but the version provided to the onAfterDeserialize method will be 2.0. Which is obviously wrong.
A fix can be to update the preconfiguredEntries in the manifest at the same time but it is not always possible (we use a CDN to deliver the code and "propose" the manifest to our clients that can deploy it several days (in the best case) later).
That's why the dataVersion entry in the manifest was good for 😉 .

@AJIXuMuK
Copy link
Collaborator

AJIXuMuK commented Jul 6, 2022

@mgrosperrin - no. As mentioned in the latest comment, after the fix on onAfterDeserialize for existing web part you will have dataVersion parameter set to the version of the web part when it was last time saved.
You don't need the dataVersion in preconfiguredEntries.

@mgrosperrin
Copy link
Author

@AJIXuMuK Not sure to understand: in the previous comment, you said that dataVersion will have the current value of the property when I add a new WebPart. So if I update the code of the WebPart without updating the manifest, I will have data from v1.0 schema and dataVersion will be 2.0.
Unless I miss something, this is not an acceptable behavior, so we will have to handle it by ourself and define our own versionning mechanism.

@AJIXuMuK
Copy link
Collaborator

AJIXuMuK commented Jul 7, 2022

@mgrosperrin - if you update dataVersion and have new properties you for sure want these new properties in the manifest, yes.

@mgrosperrin
Copy link
Author

@AJIXuMuK yes I want them, but we are an ISV that sell our WebParts and some of our clients want to update the manifest themself.
Anyway, we will implement our own mechanism to deal with versionning of the data of the WebPart since there is no more such mechanism natively.
Thanks for your time.

@AJIXuMuK
Copy link
Collaborator

AJIXuMuK commented Jul 7, 2022

What do you mean "update their own manifest"?
Manifest is the part of your sppkg file.
If you update your web part, introducing new properties and dataVersion, you can also update properties in the manifest to match new dataVersion.
Then your client updates the solution.
For new added web parts the properties will be read from the manifest (your updated manifest).
For existing ones you will have onAfterDeserialize to check the current saved dataVersion and "migrate" if needed

I don't quite get what scenarios are not covered not this approach. properties in the manifest are in sync with the current dataVersion and you have an entry point for migration

@mgrosperrin
Copy link
Author

mgrosperrin commented Jul 7, 2022

The scenario not covered is the time between the update of the code of the WebPart (that we deploy on a CDN) and the time our clients update the sppkg file we send to them. During this interval, WebPart added to the page will have the "old properties" with no way to know they are old (because the dataVersion will be the new one).
For some of our clients, this interval vary from days to weeks (to not say months). See the first paragraph of this comment #1522 (comment)

To my point of view, this is the reason of the dataVersion entry in the manifest

@AJIXuMuK
Copy link
Collaborator

AJIXuMuK commented Jul 7, 2022

@mgrosperrin - I see your point.
But for that you probably want to use this.manifest.version, not dataVersion.

@AJIXuMuK AJIXuMuK reopened this Jul 7, 2022
@AJIXuMuK
Copy link
Collaborator

AJIXuMuK commented Jul 7, 2022

Reopening it for internal tracking until the code is rolled out.

@mgrosperrin
Copy link
Author

@AJIXuMuK not sure to follow you. You recommand me to use the version of the manifest? So I will have logic that will check two unrelated versions to determine the version of the schema of the data I am currently deserializing? It is really error prone and in our cases we use the manifest's version to version... the manifest (basically to know which version of the WebPart is used (title, category, etc...)

Can you explain why the dataVersion in the preconfiguredEntries of the manifest has been removed?

@AJIXuMuK
Copy link
Collaborator

AJIXuMuK commented Jul 8, 2022

@mgrosperrin - as mentioned in the issue you have posted above #1522 dataVersion is a runtime value. It was not removed from the manifest as it has never been there (at least not in GA versions).
So, for now checking the version would be something you want to do (I'd expect version of the component to be upgraded to if you change the data version. These 2 versions may mismatch but probably will have 1:1 mapping).

But your point and scenario is valid.
We'll discuss internally and maybe add the property to the manifest.
In that case in onAfterDeserialize dataVersion would be either the one from saved props (web part already on the page) or manifest.dataVersion || this.dataVersion (for new added web parts).

@mgrosperrin
Copy link
Author

@AJIXuMuK As the propery was mentioned in the documentation of the manifest (see #8214 that remove it after one of my recent issue), I (wrongly) guess it was removed for some reasons 😄

I agree with your proposal to add a version specific to the data in the manifest instead of maintaining a mapping between version of the manifest and the version of the data 'especially as we compute the manifest's version during the build step 😉 ).
Please let me know if you need any more information on this topic!

@AJIXuMuK
Copy link
Collaborator

This should be rolled out WW by now

@mgrosperrin
Copy link
Author

Hello @AJIXuMuK,
As you close this issue, is there any other issue to continue the discussion about implementing a version specific to the data in the manifest?

@AJIXuMuK
Copy link
Collaborator

@mgrosperrin - yes, please create a new one

@ghost
Copy link

ghost commented Jul 28, 2022

Issues that have been closed & had no follow-up activity for at least 7 days are automatically locked. Please refer to our wiki for more details, including how to remediate this action if you feel this was done prematurely or in error: Issue List: Our approach to locked issues

@ghost ghost locked as resolved and limited conversation to collaborators Jul 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area:spfx Category: SharePoint Framework (not extensions related) status:fixed-next-drop Issue planned to be fixed in an upcoming release. type:bug-suspected Suspected bug (not working as designed/expected). See “type:bug-confirmed” for confirmed bugs.
Projects
None yet
Development

No branches or pull requests

2 participants