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

build: dependency upgrades and dependency instructions update #38

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

nathanross
Copy link

@nathanross nathanross commented Oct 21, 2023

The purpose of this PR is to re-enable development on this repo
by upgrading dependencies and resolving all blockers to the
instructions for "Get Going" on development in the README

1. upgrade dependencies

fixes the following:

  • yarn tauri dev was failing to compile due to out of date dependencies
  • general benefits of update of dependencies (features, bugfixes, etc)

default build fail

It is true that create-react-app is no longer maintained as a react framework and is instead gearing towards becoming more of a launcher - because of this @craco/craco can be considered inherently an outdated dependency. Adopting a maintained react framework would be a great project for another pull request but for simplicity is out of scope of this PR.

2. add @babel/plugin-proposal-private-property-in-object explicitly as a dependency

fixes the following:

  • warning on yarn tauri dev and other commands that this required
    dependency is not declared properly, risking of package breakage.

babel warning

3. Upgrade attributes for chaktra UI components.

fixes the following:

  • Various errors caused by upgrade of Chakra UI to latest version

attributeFailures1
attributeFailures2

4. Updated a broken link in the development setup instructions

  • Updated link for tauri setup in README as current link is broken

The purpose of this commit is to re-enable development on this repo
by upgrading dependencies and resolving all blockers to the
instructions for "Get Going" on development in the README

1. upgrade dependencies

fixes:
- yarn tauri dev was failing to compile due to outdated dependencies
- general outdatedness of dependencies that makes finding relevant
documentation difficult and risks adding tech debt on every change.

2. @babel/plugin-proposal-private-property-in-object

fixes:
- warning on yarn tauri dev and other commands that this required
dependency is not declared properly, risking of package breakage.

3. Upgrade attributes for chaktra UI components.

fixes:
- Various errors caused by upgrade of Chakra UI to latest version

4. Updated a broken link in the development setup instructions

- Updated link for tauri setup.
@nathanross nathanross changed the title fix: dependency upgrades and dependency instructions update build: dependency upgrades and dependency instructions update Oct 21, 2023
@nathanross
Copy link
Author

@simon-wh Pinging to put on your radar for your review on a convenient day.

If you identify any improvements needed during the workweek, I will happily address but notifying in advance that I will typically be unable to get to them until the weekend following.

@simon-wh
Copy link
Member

Hey @nathanross, sorry for the delay. I'll aim to give you a proper review this week, if not, next week, so it can get nicely wrapped up :)

Thanks for the PR!

"react-piano": "^3.1.3",
"react-scripts": "^4.0.3",
"react-scripts": "^5.0.1",
"tauri": "^0.15.0",

Choose a reason for hiding this comment

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

The "tauri" dependency should not be included here, it's an alpha channel of the "@tauri-apps/cli" that is already included, and the alpha seems to have been abandoned some 9 months ago by now and so is out of date in addition to not being needed.

Copy link
Author

@nathanross nathanross Apr 28, 2024

Choose a reason for hiding this comment

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

You're right - thanks, and sorry for the long time it took to reply.

Comment on lines -45 to 49
index={1}
noChildren={entry.notes?.length ?? 0}
gridRow={`2 / span ${(entry.notes?.length ?? 0) + 1}`}
htmlFor={key}
>
{HIDCodes[parseInt(key)]}
{entry.notes?.length ?? HIDCodes[parseInt(key)]}
</chakra.label>
Copy link
Member

Choose a reason for hiding this comment

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

This change doesn't make much sense to me, it doesn't really seem equivalent?

Copy link
Author

Choose a reason for hiding this comment

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

@simon-wh you're right, it doesn't make any sense. Sorry for the long absence - in my defense I became a father a few months ago. I'll rework this PR next weekend.

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