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

Sample package-lock fixes & migration to vite for react samples #200

Merged
merged 18 commits into from
Nov 30, 2022

Conversation

ryanbliss
Copy link
Contributor

In this pull request, I've addressed numerous issues with the samples that stem from package-lock files that included references to private npm feeds as well as dependencies that aren't actually in package.json files. Here are the changes included in this PR:

  1. Deleted all package-lock files from each sample
  2. Deleted lerna-package-lock from root directory
  3. Ran fresh installs from each to generate new package-lock files
  4. Resolved webpack 5 issues for buffer references used in Fluid Framework
  5. To fix above for React samples, I migrated React samples to Vite, which was not only a simpler solution but also improves build times considerably
  6. Added eslint dependencies and .eslintrc files to each sample to resolve issues when running samples from each individual sample, which incidentally also runtime lint errors which were previously uncaught. Those issues have now been fixed.
  7. Minor bug fixes / console error resolutions that I found when testing samples.

To make sure this doesn't cause any regressions, I tested each sample using npm run start and npm run build, both in each individual sample and from the root directory. Each appears to be working as expected!

Copy link
Collaborator

@corinagum corinagum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM ~

Copy link
Contributor

@pradeepananth pradeepananth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the samples, especially react-media-template, tested in Teams?
Please share any screen shot of sample running in Teams.

@ryanbliss
Copy link
Contributor Author

Are the samples, especially react-media-template, tested in Teams? Please share any screen shot of sample running in Teams.

Sure, I just tested in Teams with Ngrok, and actually found a quirk with how ngrok and vite work. I had to add a new start-https command which basically lets Vite work with https during hot reloading. For localhost, start works just fine. This has no impact on blob deployments. I've also updated the README with this info and will investigate ways to remove this requirement. Note: this only impacts the React samples.

image

@ryanbliss ryanbliss merged commit e44d257 into main Nov 30, 2022
@ryanbliss ryanbliss deleted the ryanbliss/package-lock-npmrc-fixes branch November 30, 2022 01:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[JS Bug]: Error when trying to run "npm install" in javascript/21.react-media-template
4 participants