Skip to content
This repository has been archived by the owner on Jun 9, 2023. It is now read-only.

chore: remove yarn and only use npm #461

Merged
merged 5 commits into from
Mar 31, 2021

Conversation

ojeytonwilliams
Copy link
Contributor

  • I have read Chapter's contributing guidelines.
  • My pull request has a descriptive title (not a vague title like Update README.md).
  • My pull request targets the master branch of Chapter.

Closes #459

Before this PR, npm was used to install packages both in manual development and when using Docker, but yarn was used for scripts. I've removed yarn entirely and re-written the scripts to use npm run instead of yarn.

The objective is just to simplify development by only relying on a single package manager.

@ojeytonwilliams ojeytonwilliams added the Documentation Improvements or additions to documentation label Mar 25, 2021
@gitpod-io
Copy link

gitpod-io bot commented Mar 25, 2021

@allella
Copy link
Contributor

allella commented Mar 25, 2021

@paulywill @pdotsani @paizle this PR looks to remove Yarn and use only npm. You were involved in yarn issues / conversations on #402 and #447 so I though you might want to give a look too.

@ojeytonwilliams
Copy link
Contributor Author

Since you're developing on Windows, @paizle, it would be great if you could confirm I've not broken the commands.

@paizle
Copy link
Contributor

paizle commented Mar 26, 2021

I will definitely have have a look.

@paizle
Copy link
Contributor

paizle commented Mar 28, 2021

Ok, I checked this out and the docker method works (docker-compose up).

I tried the manual way and ran into some issues but I was able to fix them on my side.

In packackage.json:
I replaced the single quotes:

"both": "concurrently 'npm run dev' 'cd client && npm run dev'",

With escaped double quotes:

"both": "concurrently \"npm run dev\" \"cd client && npm run dev\"",

and I was able to run the app with "npm run both".

I'm still not sure on exactly why "npm run db:reset" or the other db commands don't work on my platform:

Error output from running "npm run db:reset":

ts-node -P tsconfig.server.json ./node_modules/.bin/typeorm "schema:drop"

\Chapter\node_modules.bin\typeorm:2
basedir=$(dirname "$(echo "$0" | sed -e 's,\,/,g')")
^^^^^^^

SyntaxError: missing ) after argument list

I've run into this problem before on my Git Bash for Windows platform and solved it by replacing the line in package.json:

"typeorm": "ts-node -P tsconfig.server.json ./node_modules/.bin/typeorm",

to

"typeorm": "ts-node -P tsconfig.server.json ./node_modules/typeorm/cli.js",

I don't expect this to impact other platforms (though I could be wrong). I researched it months ago.

I'll paste my modified package.json here:

{
  "name": "chapter",
  "version": "0.0.1",
  "description": "A self-hosted event management tool for nonprofits",
  "main": "server/index.js",
  "directories": {
    "doc": "docs"
  },
  "scripts": {
    "dev": "ts-node-dev --no-notify -P tsconfig.server.json ./server/app.ts",
    "typeorm": "ts-node -P tsconfig.server.json ./node_modules/typeorm/cli.js",
    "db:generate": "npm run typeorm migration:generate -- -n",
    "db:drop": "npm run typeorm schema:drop",
    "db:seed": "ts-node -P tsconfig.server.json ./db/generator/index.ts",
    "db:migrate": "npm run typeorm migration:run",
    "db:reset": "npm run db:drop && npm run db:migrate && npm run db:seed",
    "both": "concurrently \"npm run dev\" \"cd client && npm run dev\"",
    "build:client": "cd client && npm run build",
    "start": "ts-node -P tsconfig.server.json ./server/production.ts",
    "production": "npm run build:client && npm start",
    "test": "npx jest --coverage --verbose",
    "test:watch": "npx jest --watchAll",
    "postinstall": "node scripts/postInstall.js",
    "lint": "eslint './**/*.{ts,tsx,js,jsx}'",
    "lint:fix": "eslint './**/*.{ts,tsx,js,jsx}' --fix",
    "pretty": "prettier --write client/**/*.ts* server/**/*.ts"
  },
  "repository": {
    "type": "git",
    "url": "git+https://github.com/freeCodeCamp/chapter.git"
  },
  "keywords": [
    "chapter",
    "meetup",
    "open-source"
  ],
  "author": "author@chapter.io",
  "license": "BSD-3-Clause",
  "bugs": {
    "url": "https://github.com/freeCodeCamp/chapter/issues"
  },
  "homepage": "https://github.com/freeCodeCamp/chapter#readme",
  "dependencies": {
    "@types/cors": "^2.8.7",
    "apollo-server-express": "^2.16.1",
    "babel-plugin-module-resolver": "^3.2.0",
    "babel-plugin-transform-typescript-metadata": "^0.2.2",
    "class-validator": "^0.12.2",
    "cors": "^2.8.5",
    "cross-fetch": "^3.0.4",
    "dotenv": "^8.2.0",
    "express": "^4.17.1",
    "express-async-handler": "^1.1.4",
    "express-response-errors": "^1.0.4",
    "faker": "^4.1.0",
    "fork-ts-checker-webpack-plugin": "^1.5.1",
    "graphql": "^15.3.0",
    "is-docker": "^2.0.0",
    "morgan": "^1.9.1",
    "next": "^9.5.1",
    "nodemailer": "^6.3.1",
    "pg": "^8.0.3",
    "pg-hstore": "^2.3.3",
    "react": "^16.13.1",
    "react-dom": "^16.13.1",
    "reflect-metadata": "^0.1.13",
    "swagger-ui-express": "^4.1.3",
    "ts-node": "^8.6.2",
    "tsconfig-paths": "^3.9.0",
    "type-graphql": "^1.0.0-rc.3",
    "typeorm": "^0.2.20",
    "typescript": "^3.9.7"
  },
  "devDependencies": {
    "@babel/plugin-proposal-decorators": "^7.8.3",
    "@babel/plugin-proposal-nullish-coalescing-operator": "^7.8.3",
    "@testing-library/react": "^9.3.0",
    "@types/bluebird": "^3.5.28",
    "@types/chai": "^4.2.4",
    "@types/express": "^4.17.1",
    "@types/faker": "^4.1.8",
    "@types/get-port": "^4.2.0",
    "@types/jest": "^25.1.4",
    "@types/morgan": "^1.7.37",
    "@types/next": "^8.0.6",
    "@types/node": "^14.14.6",
    "@types/nodemailer": "^6.2.2",
    "@types/pg": "^7.14.5",
    "@types/react": "^16.9.9",
    "@types/react-dom": "^16.9.2",
    "@types/sinon-chai": "^3.2.3",
    "@types/supertest": "^2.0.8",
    "@types/swagger-ui-express": "^4.1.1",
    "@types/validator": "^10.11.3",
    "@typescript-eslint/eslint-plugin": "^2.4.0",
    "@typescript-eslint/parser": "^2.4.0",
    "chai": "^4.2.0",
    "concurrently": "^5.2.0",
    "cross-env": "^6.0.3",
    "eslint": "^6.8.0",
    "eslint-config-prettier": "^6.4.0",
    "eslint-config-react": "^1.1.7",
    "eslint-plugin-jest": "^22.20.0",
    "eslint-plugin-prettier": "^3.1.1",
    "eslint-plugin-react": "^7.16.0",
    "get-port": "^5.0.0",
    "husky": "^3.0.9",
    "jest": "^25.1.0",
    "lint-staged": "^9.4.2",
    "nodemon": "^1.19.4",
    "prettier": "^1.19.1",
    "sinon": "^7.5.0",
    "sinon-chai": "^3.3.0",
    "supertest": "^4.0.2",
    "ts-jest": "^24.1.0",
    "ts-node-dev": "^1.0.0-pre.56"
  },
  "lint-staged": {
    "*.{ts,tsx,js,jsx}": [
      "npm run pretty",
      "git add"
    ]
  },
  "husky": {
    "hooks": {
      "pre-commit": "lint-staged"
    }
  },
  "engines": {
    "node": ">=14.15.0",
    "npm": ">=6.14.8"
  }
}

package.json Outdated Show resolved Hide resolved
@ojeytonwilliams
Copy link
Contributor Author

@paizle thanks for going through all this. Just to confirm: was db:reset working for you before this PR?

@paizle
Copy link
Contributor

paizle commented Mar 29, 2021

@paizle thanks for going through all this. Just to confirm: was db:reset working for you before this PR?

No, the DB commands did not work. The only thing I did to get them to work was changing:

"typeorm": "ts-node -P tsconfig.server.json ./node_modules/.bin/typeorm",

to

"typeorm": "ts-node -P tsconfig.server.json ./node_modules/typeorm/cli.js",

In package.json.

package.json Show resolved Hide resolved
@allella
Copy link
Contributor

allella commented Mar 29, 2021

@paizle 's issue seems to be related to Git Bash + Windows and probably because there's a symlink involved.

image

@ojeytonwilliams do we think using the literal path instead of a symlink is likely to fix one environment and break others? If so, perhaps we break that into a new issue.

I found some chat about the escaping of characters being an issue / bug with Git Bash on Windows so symlink and/or escaping seems like they could the what's breaking Git Bash.

@allella
Copy link
Contributor

allella commented Mar 29, 2021

Or, @paizle have you checked if enabling symlinks will work in Git Bash?

@paizle
Copy link
Contributor

paizle commented Mar 29, 2021

Or, @paizle have you checked if enabling symlinks will work in Git Bash?

This rings a bell from when I was looking into it before. I will check it out.

@allella
Copy link
Contributor

allella commented Mar 29, 2021

Windows doesn't natively support symlinks, so it's rather likely this could fix Git Bash. If so, we can document the exception for Git Bash because changing the typeorm path is likely to cause issues for some other environment or configuration.

@ojeytonwilliams
Copy link
Contributor Author

@allella ah, that makes a lot of sense.

And, yes, let's handle this in a separate issue. I'll create one to track this and revert the last commit.

@allella
Copy link
Contributor

allella commented Mar 30, 2021

@ojeytonwilliams I did a ton of big and small changes to the CONTRIBUTING.md in #466 and I suspect that will leave us with a gnarly merge conflict when things are merge. Not sure the best way to do it with the two open PRs, but wanted to give a heads up.

The Git Bash issues mentioned above and further in #465 don't seem to be specific to npm vs yarn, so I think we can ignore that as a concern.

CONTRIBUTING.md provides notes on requiring / installing Node.js, so that means we know everyone will have npm by default. Moving everything to npm will at least avoid "yarn not installed" errors.

If it turns out there was a good reason to use yarn for some, or all of the things, we can always just reverse things.

You good with proceeding?

@ojeytonwilliams
Copy link
Contributor Author

Thanks for the heads up and yeah, I'm fine with proceeding. Just give me a shout if you merge either one - I'd be happy to fix any conflicts.

allella added a commit to allella/chapter that referenced this pull request Mar 31, 2021
@allella
Copy link
Contributor

allella commented Mar 31, 2021

@ojeytonwilliams #466 did a find and replace on yarn to npm.

If you revert your CONTRIBUTING.md changes in this PR, then I think we're ready to commit it.

Thanks

@ojeytonwilliams
Copy link
Contributor Author

Actually, a straight search and replace doesn't quite work, since yarn db:reset -> npm run db:reset and so on. I'll fix that in a sec.

@ojeytonwilliams
Copy link
Contributor Author

@allella all done, should be good to go now.

@allella allella merged commit 4d1c561 into freeCodeCamp:master Mar 31, 2021
@ojeytonwilliams ojeytonwilliams deleted the chore/yarn-to-npm branch March 31, 2021 18:11
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Yarn missing as a dependency or devDependency
3 participants