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

Feat: Resolved booking mismatch & updated react to CR #30

Merged
merged 7 commits into from
Dec 14, 2024

Conversation

Dunkstormen
Copy link
Contributor

@Dunkstormen Dunkstormen commented Dec 12, 2024

Updating packages, dependencies etc. for React 19.

Fixed sort and merging of online/booked positions in the BookingComponent which solves #21.

image

@bjerrecs bjerrecs changed the title React 19 & fixed to BookingComponent Feat: Resolved booking mismatch & updated react to CR Dec 12, 2024
@bjerrecs bjerrecs requested review from bjerrecs and blt950 and removed request for bjerrecs December 12, 2024 21:02
@bjerrecs bjerrecs removed the request for review from blt950 December 14, 2024 09:58
@blt950
Copy link
Member

blt950 commented Dec 14, 2024

This is amazing ❤️ I'll review it locally when we have the event this evening to check if it works as intended, if so - merge it

@Dunkstormen
Copy link
Contributor Author

Dunkstormen commented Dec 14, 2024

This is amazing ❤️ I'll review it locally when we have the event this evening to check if it works as intended, if so - merge it

Just worked a bit more on the component and I'm refactoring/rewriting the code for better readability etc. (Will come in a later PR). Just keep an eye on react-snowfall. It worked when I did this PR then when I moved from Codespaces to local environment it kept throwing dependency errors, as it seems the package.json for it isn't updated.

"peerDependencies": { "react": "^16.8 || 17.x || 18.x", "react-dom": "^16.8 || 17.x || 18.x" },

I've however tested it with react-19 without issues. May make a fork of it to update the dependencies.

@blt950
Copy link
Member

blt950 commented Dec 14, 2024

@Dunkstormen Could you use https://cc.vatsim-scandinavia.org/api/positions instead of the position.json (can be deleted). It was only intended temporarily while we waited for API functionality that is now available.

@Dunkstormen
Copy link
Contributor Author

@blt950 I can see my latest commit has sneaked in here. Just so you're aware it refs the new component which is not fully done yet. So please either ignore latest commit or wait for me to be done with the full refactoring.

@blt950
Copy link
Member

blt950 commented Dec 14, 2024

@Dunkstormen No problem. Everything seems to work as of now. So I'm merging this and then refactor can be it's own PR

@blt950 blt950 merged commit 9788bb3 into Vatsim-Scandinavia:main Dec 14, 2024
@blt950
Copy link
Member

blt950 commented Dec 14, 2024

I think we need to fix the dependencies @Dunkstormen , the build failed

npm error code ERESOLVE
npm error ERESOLVE could not resolve
npm error
npm error While resolving: [email protected]
npm error Found: [email protected]
npm error node_modules/react
npm error   react@"^19.0.0" from the root project
npm error   peer react@"^17.0.2 || ^1[8](https://github.com/Vatsim-Scandinavia/home/actions/runs/12332767193/job/34421008007#step:6:9).0.0 || ^19.0.0" from @astrojs/[email protected]
npm error   node_modules/@astrojs/react
npm error     @astrojs/react@"^4.1.0" from the root project
npm error   17[9](https://github.com/Vatsim-Scandinavia/home/actions/runs/12332767193/job/34421008007#step:6:10) more (@floating-ui/react-dom, @nextui-org/accordion, ...)
npm error
npm error Could not resolve dependency:
npm error peer react@"^16.8 || 17.x || 18.x" from [email protected]
npm error node_modules/react-snowfall
npm error   react-snowfall@"^2.2.0" from the root project
npm error
npm error Conflicting peer dependency: [email protected]
npm error node_modules/react
npm error   peer react@"^16.8 || 17.x || 18.x" from [email protected]
npm error   node_modules/react-snowfall
npm error     react-snowfall@"^2.2.0" from the root project
npm error
npm error Fix the upstream dependency conflict, or retry
npm error this command with --force or --legacy-peer-deps
npm error to accept an incorrect (and potentially broken) dependency resolution.
npm error
npm error
npm error For a full report see:
npm error /home/runner/.npm/_logs/2024-12-14T20_[10](https://github.com/Vatsim-Scandinavia/home/actions/runs/12332767193/job/34421008007#step:6:11)_35_375Z-eresolve-report.txt
npm error A complete log of this run can be found in: /home/runner/.npm/_logs/2024-[12](https://github.com/Vatsim-Scandinavia/home/actions/runs/12332767193/job/34421008007#step:6:13)-14T20_10_35_375Z-debug-0.log

@Dunkstormen
Copy link
Contributor Author

I think we need to fix the dependencies @Dunkstormen , the build failed

npm error code ERESOLVE
npm error ERESOLVE could not resolve
npm error
npm error While resolving: [email protected]
npm error Found: [email protected]
npm error node_modules/react
npm error   react@"^19.0.0" from the root project
npm error   peer react@"^17.0.2 || ^1[8](https://github.com/Vatsim-Scandinavia/home/actions/runs/12332767193/job/34421008007#step:6:9).0.0 || ^19.0.0" from @astrojs/[email protected]
npm error   node_modules/@astrojs/react
npm error     @astrojs/react@"^4.1.0" from the root project
npm error   17[9](https://github.com/Vatsim-Scandinavia/home/actions/runs/12332767193/job/34421008007#step:6:10) more (@floating-ui/react-dom, @nextui-org/accordion, ...)
npm error
npm error Could not resolve dependency:
npm error peer react@"^16.8 || 17.x || 18.x" from [email protected]
npm error node_modules/react-snowfall
npm error   react-snowfall@"^2.2.0" from the root project
npm error
npm error Conflicting peer dependency: [email protected]
npm error node_modules/react
npm error   peer react@"^16.8 || 17.x || 18.x" from [email protected]
npm error   node_modules/react-snowfall
npm error     react-snowfall@"^2.2.0" from the root project
npm error
npm error Fix the upstream dependency conflict, or retry
npm error this command with --force or --legacy-peer-deps
npm error to accept an incorrect (and potentially broken) dependency resolution.
npm error
npm error
npm error For a full report see:
npm error /home/runner/.npm/_logs/2024-12-14T20_[10](https://github.com/Vatsim-Scandinavia/home/actions/runs/12332767193/job/34421008007#step:6:11)_35_375Z-eresolve-report.txt
npm error A complete log of this run can be found in: /home/runner/.npm/_logs/2024-[12](https://github.com/Vatsim-Scandinavia/home/actions/runs/12332767193/job/34421008007#step:6:13)-14T20_10_35_375Z-debug-0.log

@blt950 I've made a PR on the react-snowfall repo. Hoping it may be updated. Otherwise we may fallback to copying the source code into the project. Will look further into once I'm fully done with the rewrite of the BookingComponent. Only missing calling the new API endpoint which I want to avoid calling each time we're trying to match a callsign to a position.

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.

3 participants