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

get app running locally #332

Closed
wants to merge 16 commits into from
Closed

get app running locally #332

wants to merge 16 commits into from

Conversation

charkour
Copy link
Collaborator

@charkour charkour commented Jun 4, 2023

Hi @kvlinden and @rpruim. I have some free time this summer, so I'd like to modernize and improve the third-party dependencies for this project so you and the students can more easily add features in the future. If you'd like to talk in more detail, please email me. Thank you

First order of business is to actually run the app locally with modern a modern JS runtime (Node v18)
This fixes bugs to allow the app to run locally with Node v18 and PNPM v8. This updates FullCalendar to v6 to fix some vdom issues.

To update to Node v18, use Node Version Manager
To use pnpm, follow their docs

Many of the updates were due to ESLint rule changes and auto-formatting.

  • Update Readme

@charkour charkour marked this pull request as draft June 4, 2023 23:39
"hooks": {
"pre-commit": "lint-staged"
}
"hooks": {}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed husky hook for the time being

@@ -7,9 +7,9 @@ module.exports = {
},
extends: [
"react-app",
"airbnb",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ESLint was causing errors. It's very outdated so for now, we'll work without a full eslint config.

Comment on lines +11 to +40
enum SemesterLength {
Full = "Full",
HalfFirst = "First",
HalfSecond = "Second",
IntensiveA = "A",
IntensiveB = "B",
IntensiveC = "C",
IntensiveD = "D",
}

enum Term {
Fall = "FA",
Spring = "SP",
// eslint-disable-next-line typescript-sort-keys/string-enum
Interim = "IN", // TODO: Remove?
Summer = "SU",
}


/* Used for containing the global state of the app
and a dispatcher to perform updates against the
state of the app.
*/
export const AppContext = createContext<AppContext>({
appDispatch: voidFn,
appState: initialAppState,
appState: {
classes: [],
colorBy: 0,
constraints: {},
departments: [],
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

For the time being, this is duplicated.

@charkour
Copy link
Collaborator Author

charkour commented Jun 5, 2023

Tests faill because of switching to PNPM

@charkour charkour marked this pull request as ready for review June 5, 2023 00:06
@charkour charkour requested review from rpruim and kvlinden June 5, 2023 00:07
@kvlinden
Copy link
Collaborator

Thanks for this, Charles. Alas, we've been buried with summer projects and the new MDS program going live next month. We will be fielding a Schedulizer student team in early September, so hopefully we'll have some time to resurrect this system and review your proposed changes.

@charkour
Copy link
Collaborator Author

Roger that. I can verify my changes work and merge them in. That way there's no hassle for the students this fall.

@kvlinden
Copy link
Collaborator

@charkour - I'm finally coming back around to this issue and am having trouble getting the system running locally. I haven't done active development on this system for a while, so I'm probably forgetting some key steps.

I verified that I've got Node 18.17.1; NPM 10.0.0; PNPM 8.6.0 on WSL2 Linux, and then I tried running the system on both the production and then the fix-local branches.

  • npm install worked on production but npm start failed to bring up the system (citing errors in &fullcalendar)
  • npm install failed on fix-local (citing errors with eslint).

(naive?) question: is npm-install and then npm-start the right way to start the local server?

@charkour
Copy link
Collaborator Author

npm install worked on production but npm start failed to bring up the system (citing errors in &fullcalendar)

Yup! I ran into that too. This branch aims to fix those issues.

npm install failed on fix-local (citing errors with eslint).

The ESLint errors should be fixable, so you'll need to do that quickly. The console will display what needs to be updated.

The correct way to run this is running the commands pnpm install then pnpm start in the product directory with the package.json file.

@charkour
Copy link
Collaborator Author

Please feel free to push to this branch. I cannot confidently commit time to dev work, but I'll advise as possible.

@kvlinden
Copy link
Collaborator

Thanks - I'm not familiar with pnpm.
It's running now, but generating an error about installing @testing-library/react. I can work on that.

@charkour charkour closed this Dec 18, 2024
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.

4 participants