-
Notifications
You must be signed in to change notification settings - Fork 833
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 InjectCSS sample from SPFx v1.8.0 to v1.20.0 #1416
Conversation
Thank you for opening a PR to our samples repo and helping us upgrade our project 👍 |
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.
@tom-daly Awesome work 👏👏👏
I had a quick check and managed to build and deploy the solution on my tenant. You Rock 🤩
I noticed a few old dependencies which should be aligned with SPFx version. I know the extension does not use them but since they are in the packge.json file we may fix them up as well. Please do give them a double check before we proceed 🙏
this kind of info you may always catch using the validate
action using SPFx Toolkit or m365 spfx project doctor
command using CLI for Microsoft 365.
This was the only 1 of my commits where I used the m365 cli to check the upgrade and it didn't work well. I also ran the dr and it did not pick up your suggestions but just remove gulp and add gulp-cli. I've taken the package dependencies from a clean v1.20.0 project and will be checking that in momentarily. |
I used SPFx Toolkit to support me in this review and the validate action picked up this as well. |
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.
@tom-daly thanks for the quick turn around.
I think we are almost there. Let's do a bit of clean up and we should be ready to merge 🚀
package-lock.json
Outdated
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.
I think we should not update the package-lock.json which is outside of the sample folder.
Lets revert this
@tom-daly are you still working on this PR? If it is ready to be merged please do let us know by hitting the |
@tom-daly Awesome work 👏👏👏👏 |
What's in this Pull Request?
These changes upgrade the solution from SPFx v1.8.0 to v1.20.0.
It also fixes the warning from the linter by adding the proper typing
It also includes modified instructions on how to deploy using PnP PowerShell since the new instructions Sept 9, 2024.