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

Switch to pnpm #1071

Merged
merged 5 commits into from
Sep 28, 2023
Merged

Switch to pnpm #1071

merged 5 commits into from
Sep 28, 2023

Conversation

axelboc
Copy link
Collaborator

@axelboc axelboc commented Sep 28, 2023

https://pnpm.io

Lots of benefits to pnpm:

  • Much, much faster than npm
  • Lighter node_modules when working on multiple projects, since packages are symbolically linked to a global store
  • No phantom dependencies: the node_modules folder is structured in such a way that a package can only "see" dependencies that it depends on explicitly. In our case, the prop-types package, while used in the codebase, was not actually installed as a direct dev dependency. Phantom dependencies can cause more complicated issues when various (sub-)dependencies dependent on multiple versions of the same package.
  • Lots of ways to resolve peer dependencies issues, outdated packages, etc. via the pnpm property in package.json. No more --legacy-peer-deps, ever.

Comments incoming, as always.

Comment on lines +54 to +58
- name: Install pnpm
uses: pnpm/action-setup@v2
with:
version: 8.x

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I've replicated the set-up of other front-end projects, but maybe there's a better way to install it in MXCuBE. 🤷

@@ -156,55 +157,51 @@ pip install -e .
pip install -e .
```

The above makes it possible to add break points directly in the "checked out code".
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Repeats what is said before the commands block.

@@ -156,55 +157,51 @@ pip install -e .
pip install -e .
```

The above makes it possible to add break points directly in the "checked out code".

Before running any test, make sure that the local *Redis* server is running. For example with the `mxcubeweb` *conda* environment activated in a terminal, run the `redis-server` command.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moved to next section.

conda activate mxcubeweb
mxcubeweb-server -r $(pwd)/mxcubeweb/test/HardwareObjectsMockup.xml/ --static-folder $(pwd)/mxcubeweb/ui/build/ -L debug
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

At this point, we're already in the mxcubeweb folder (there's a cd mxcubeweb somewhere above).

pnpm start

# Note that you can also run any pnpm script from the root folder with:
pnpm --prefix ui <script>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thought it'd be a good place to explain this.

ui/README.md Outdated
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is the default README of create-react-app; I don't think it's needed.

"ws": "^8.5.0"
},
"devDependencies": {
"@fortawesome/fontawesome-free": "^5.15.4",
"@testing-library/cypress": "9.0.0",
"@testing-library/cypress": "10.0.1",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Upgrading Cypress Testing Library to fix the peer dep issue, while I'm at it.

Comment on lines +77 to +79
"@testing-library/jest-dom": "^5.16.2",
"@testing-library/react": "^12.1.2",
"@testing-library/user-event": "^13.5.0",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Moved to devDependencies along with a few others.

"@testing-library/jest-dom": "^5.16.2",
"@testing-library/react": "^12.1.2",
"@testing-library/user-event": "^13.5.0",
"babel-preset-react-app": "10.0.1",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Missing dep required for ESLint (and our linting config) to work properly when installed with pnpm

"less": "^4.1.2",
"less-watch-compiler": "^1.16.3",
"prettier": "3.0.0"
"prettier": "3.0.0",
"prop-types": "15.8.1",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

prop-types was missing

Base automatically changed from ab-fix-regex to develop September 28, 2023 13:52
@marcus-oscarsson
Copy link
Member

Very nice :)

@marcus-oscarsson
Copy link
Member

.. awesome actually :)

@marcus-oscarsson marcus-oscarsson merged commit ac1cddb into develop Sep 28, 2023
10 of 11 checks passed
@marcus-oscarsson marcus-oscarsson deleted the ab-pnpm branch September 28, 2023 13:55
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.

2 participants