Skip to content

Replace npm with yarn; add CI workflow to run TSC and tslint checks - #699

Open
laurence-myers wants to merge 12 commits into
whyboris:mainfrom
laurence-myers:yarn-and-ci
Open

Replace npm with yarn; add CI workflow to run TSC and tslint checks#699
laurence-myers wants to merge 12 commits into
whyboris:mainfrom
laurence-myers:yarn-and-ci

Conversation

@laurence-myers

Copy link
Copy Markdown
Contributor

To avoid linting issue sneaking in, particularly in the backend/server code, I've added a GitHub Action workflow to run tsc and tslint on every push.

yarn behaves better across different execution environments. In particular, npm ci failed to run in a GitHub Action, due to fsevents being a required dependency (somehow).

Example CI check:

https://github.com/laurence-myers/Video-Hub-App/runs/3578230215?check_suite_focus=true

This PR is based on #698, and includes its changes.

@whyboris

whyboris commented Sep 12, 2021

Copy link
Copy Markdown
Owner

Thank you for the contribution! I'll give it a try on Mac and Windows to see how it goes.

I remember when yarn was really ahead of npm in many ways, but it has felt like npm has caught up in terms of speed and locking dependencies 🤔

I'm not opposed to merging this -- I'll try it out first 🙇

@laurence-myers

Copy link
Copy Markdown
Contributor Author

Thank you for the contribution! I'll give it a try on Mac and Windows to see how it goes.

I remember when yarn was really ahead of npm in many ways, but it has felt like npm has caught up in terms of speed and locking dependencies 🤔

I'm not opposed to merging this -- I'll try it out first 🙇

Yep, I'd be happy to stick with npm, I just didn't know if it would be possible to fix the issue where package-lock.json lists fsevents as a required (transitive) dependency. Maybe using resolutions in package.json?

Plus, I noticed package-lock.json has a bunch of possibly invalid dependencies beginning with node_modules. I ran npm shrinkwrap, which gave a much smaller npm-shrinkwrap.json file.

If you look at the number of lines changed, this MR removes approx 30,000 lines. Maybe it's because npm-shrinkwrap.json is more thorough in how it lists dependencies, but maybe it's because there's unnecessary/invalid entries.

@whyboris

Copy link
Copy Markdown
Owner

Thank you again @laurence-myers for this PR 🙇‍♂️

I'm releasing version 3.3.0 soon. I'll re-visit this PR again in the future 🌻

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