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

Update dependencies #84

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

felipeplets
Copy link
Contributor

Update the project to run with the latest dev dependencies and remove all npm audit vulnerabilities.

@larssn
Copy link

larssn commented Mar 31, 2023

Status on getting this merged?

There's an issue deploying to Cloud Functions if you have a dependency on ts-mocha currently, due to the ts-node: 7.0.1 dependency. This PR would fix that.

Appears to have been fixed by what I assume is the CF team.

Still, I think it would be a good idea to get these dependencies updated, so issues like that don't come back.

@piotrwitek
Copy link
Owner

Thanks for sharing the feedback @larssn, have you tested the build from this branch to make sure it's working as expected?
I could merge it as soon as it's confirmed as I cannot test it myself. Thanks

@larssn
Copy link

larssn commented Mar 31, 2023

Alright, so here's how I tested it:

npm list ts-node
├─┬ firebase-functions-test@3.0.0
│ └─┬ jest@29.5.0
│   └─┬ @jest/core@29.5.0
│     └─┬ jest-config@29.5.0
│       └── ts-node@10.9.1
└─┬ ts-mocha@10.0.0
  └── ts-node@7.0.1
// The ts-node 7.0.1 was causing problems here.

npm r ts-mocha
npm i -D https://github.com/felipeplets/ts-mocha.git#update/deps

npm list ts-node
├─┬ firebase-functions-test@3.0.0
│ └─┬ jest@29.5.0
│   └─┬ @jest/core@29.5.0
│     └─┬ jest-config@29.5.0
│       └── ts-node@10.9.1 deduped
└─┬ ts-mocha@10.0.0 (git+ssh://git@github.com/felipeplets/ts-mocha.git#943ec777692a555438c3e1bef632570d92bb8922)
  └── ts-node@10.9.1
// No more ts-node 7.0.1

ts-mocha -p tsconfig.test.json test/**/*.spec.ts --reporter spec
 484 passing (79ms)

firebase deploy <...>
✔  functions: Finished running postdeploy script.
✔  Deploy complete!

Seems to work

@felipeplets
Copy link
Contributor Author

Thank you for testing @larssn.

@piotrwitek The only detail is that we would have to release a major version with this since we may break for users that don't have ts-node for any other purpose. I can write down the changelog with the details if you want, just let me know and we coordinate the launch.

@stenneepro
Copy link

stenneepro commented Jan 9, 2024

When the PR will be merged?
Can we upgrade TypeScript version to 5.x?

@felipeplets
Copy link
Contributor Author

@stenneepro I can update it and test next week. But feel free to contribute earlier if you can.

Also, for curiosity are you setting any problem because of ts-mocha when trying to upgrade to TyeScript 5? As it is a dev dependency I don't expect it to prevent you from upgrading your application.

@stenneepro
Copy link

stenneepro commented Jan 10, 2024

@felipeplets
I don't think upgrade to TypeScript 5 blocks my application.

@piotrwitek
Copy link
Owner

piotrwitek commented Jan 15, 2024

Hey @felipeplets, let me know when it's ready for release I could make an npm RC release to make users testing easier.

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.

None yet

4 participants