-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
Upgrade using @pnp/js and react js (react pnp js sample) to SPFx 1.20 #5286
Upgrade using @pnp/js and react js (react pnp js sample) to SPFx 1.20 #5286
Conversation
I find myself unable to build the solution. @gretchunkim can you check the dependencies again and make sure everything is included? |
Modified additional dev dependencies. Thanks! |
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.
@gretchunkim Awesome work so far 👏👏👏👏👏👏
I left a few comments with tiny things we should sort out before we merge. Please do give them a double check 🙏.
I also had a problem when testing the solution. It does not build
Please also update the assets/sample.json
file. We should update the updateDateTime
field and the SPFX-VERSION
metadata to 1.20.0
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.
Please add yourself as one of the contributors 👍 to the readme file
"react": "16.13.1", | ||
"react-dom": "16.13.1", | ||
"tslib": "2.3.1" | ||
}, | ||
"devDependencies": { | ||
"@microsoft/eslint-config-spfx": "^1.20.0", | ||
"@microsoft/eslint-plugin-spfx": "^1.20.0", | ||
"@microsoft/rush-stack-compiler-4.5": "^0.2.2", | ||
"@microsoft/sp-build-web": "1.20.1", |
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.
This should align with SPFx version
"@microsoft/sp-build-web": "1.20.1", | |
"@microsoft/sp-build-web": "1.20.0", |
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.
@Adam-it solution tested using "@microsoft/sp-build-web": "1.20.1" and build fails.
import { Label, PrimaryButton } from '@microsoft/office-ui-fabric-react-bundle'; | ||
import "@pnp/sp/items"; | ||
import { IItemUpdateResultData } from "@pnp/sp/items/types"; // Corrected import | ||
import { Label, Button } from "@fluentui/react-components"; |
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.
but we did not add the @fluentui
to that package.json
so that it most probably will present an error when building
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.
@Adam-it We should switch button away @microsoft and use @FluentUI instead. The solution failed to build using @microsoft/office-ui-fabric-react. There is a verbiage about incompatibility of SPfx with Fabric React version 2.x or older. (Fabric React versions 2.x or older are not supported in SharePoint Framework.)
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.
@gretchunkim thank you for a quick turn around. I noticed that during merge some unwanted changes got included in this branch from other samples Lets try to revert those or rebase against latest main so that this PR only updates a single sample |
b61e27a
to
cb95b15
Compare
What's in this Pull Request
Upgraded using @pnp/js and react js (aka react pnp js sample) to SPFx 1.20. Aligned button position (ref. Issue #5255)
Node Version
Node version used: 18.19.0
Checklist
README.md
file's Version history. For new samples, created a newREADME.md
file matching this templateREADME.md
has at least one static high-resolution screenshot (i.e. not a GIF) located in theassets
folder.README.md
contains complete setup instructions, including pre-requisites and permissions required.nvmrc
file indicating the version of Node.js