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

Use newer jwks-rsa library and its async/await functions #7305

Conversation

olleolleolle
Copy link
Contributor

@olleolleolle olleolleolle commented Mar 30, 2021

New Pull Request Checklist

Issue Description

This adds the requirement "parse-server works with Node 10 and newer" a real thing in package.json.

This PR intends to upgrade a dependency and switch from promisify to async/await.

Bumping the min Node.js requirement is merely a side-effect, but it makes also sense for another reason because Parse Server is officially only compatible (and tested) with Node >= 10.

Related issue: #7304

Approach

(I attempted to run npm install, but that happened to have a newer npm (7) which bumped the lockfileVersion. Hm. Not what I wanted to do.)

TODOs before merging

Current trouble w/ my bad use of the tests:

1) facebook limited auth adapter (using client id as string) should verify id_token
  - Error: <spyOn> : getSigningKey() method does not exist
  Usage: spyOn(<object>, <methodName>)
  • use spyOn correctly
  • Use the 2.x version of the library
  • Use the new functions in facebook auth file
  • Use the new functions in apple auth file
  • Add entry to changelog?

@olleolleolle olleolleolle force-pushed the package-json-use-node-10 branch from af8d93c to 04ecba8 Compare March 30, 2021 12:30
@codecov
Copy link

codecov bot commented Mar 30, 2021

Codecov Report

Merging #7305 (82d1018) into master (7042552) will decrease coverage by 0.01%.
The diff coverage is n/a.

❗ Current head 82d1018 differs from pull request most recent head 2757960. Consider uploading reports for the commit 2757960 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7305      +/-   ##
==========================================
- Coverage   93.93%   93.91%   -0.02%     
==========================================
  Files         181      179       -2     
  Lines       13194    13168      -26     
==========================================
- Hits        12394    12367      -27     
- Misses        800      801       +1     
Impacted Files Coverage Δ
src/batch.js 91.37% <0.00%> (-1.73%) ⬇️
src/Adapters/Files/GridFSBucketAdapter.js 79.50% <0.00%> (-0.82%) ⬇️
src/Routers/FilesRouter.js 88.28% <0.00%> (-0.27%) ⬇️
...dapters/Storage/Postgres/PostgresStorageAdapter.js 95.36% <0.00%> (-0.16%) ⬇️
src/ParseServer.js 89.77% <0.00%> (-0.12%) ⬇️
src/cli/utils/commander.js 100.00% <0.00%> (ø)
src/ParseServerRESTController.js 98.50% <0.00%> (ø)
src/Deprecator/Deprecations.js
src/Deprecator/Deprecator.js
src/Adapters/Storage/Mongo/MongoStorageAdapter.js 93.24% <0.00%> (+0.65%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7042552...2757960. Read the comment docs.

@mtrezza
Copy link
Member

mtrezza commented Mar 30, 2021

To clarify for reviewers, this PR intents to upgrade a dependency and switch from promisify to async/await.

Bumping the min Node.js requirement is merely a side effect, but it makes also sense for another reason because Parse Server is officially only compatible (and tested) with Node >= 10.

@olleolleolle Could you please change the PR issue and approach to be specific to the referring issue to prevent any confusion?

@olleolleolle olleolleolle marked this pull request as draft March 30, 2021 14:21
@olleolleolle olleolleolle changed the title package.json: Require Node 10+ Use newer jwks-rsa library and its async/await functions Mar 30, 2021
@olleolleolle olleolleolle force-pushed the package-json-use-node-10 branch from 04ecba8 to 871e4c5 Compare March 30, 2021 15:06
@olleolleolle olleolleolle marked this pull request as ready for review March 30, 2021 15:07
@olleolleolle olleolleolle force-pushed the package-json-use-node-10 branch from 871e4c5 to a6cba61 Compare March 30, 2021 15:18
@olleolleolle
Copy link
Contributor Author

olleolleolle commented Mar 30, 2021

Ah, perfect, the right tests are failing, I'll get to that.

(I can see how I missed them locally: they are xited there. They do not run, by default. Will investigate.)

This pushes up to declare Node 10+ as a requirement, but we were using
that in practice, before, too.

jwks-rsa CHANGELOG mentions this upgrade guide:
https://github.com/auth0/node-jwks-rsa/blob/master/CHANGELOG.md#migrated-callbacks-to-asyncawait
@olleolleolle olleolleolle force-pushed the package-json-use-node-10 branch from b52b65b to 85fb56b Compare April 1, 2021 15:13
@olleolleolle
Copy link
Contributor Author

olleolleolle commented Apr 29, 2021

@mtrezza 👋 Hi, I know that I won't have time to get the tests in shape in the forseeable future, so please do investigate.

Acceptable alternative: close this PR.

@mtrezza
Copy link
Member

mtrezza commented Apr 29, 2021

Thanks for the update, let's leave this open for now, if someone wants to pick this up.

@mtrezza
Copy link
Member

mtrezza commented Sep 3, 2021

⚠️ Important change for merging PRs from Parse Server 5.0 onwards!

We are planning to release the first beta version of Parse Server 5.0 in October 2021.

If a PR contains a breaking change and is not merged before the beta release of Parse Server 5.0, it cannot be merged until the end of 2022. Instead it has to follow the Deprecation Policy and phase-in breaking changes to be merged during the course of 2022.

One of the most voiced community feedbacks was the demand for predictability in breaking changes to make it easy to upgrade Parse Server. We have made a first step towards this by introducing the Deprecation Policy in February 2021 that assists to phase-in breaking changes, giving developers time to adapt. We will follow-up with the introduction of Release Automation and a branch model that will allow breaking changes only with a new major release, scheduled for the beginning of each calendar year.

We understand that some PRs are a long time in the making and we very much appreciate your contribution. We want to make it easy for PRs that contain a breaking change and were created before the introduction of the Deprecation Policy. These PRs can be merged with a breaking change without being phased-in before the beta release of Parse Server 5.0. We are making this exception because we appreciate that this is a time of transition that requires additional effort from contributors to adapt. We encourage everyone to prepare their PRs until the end of September and account for review time and possible adaptions.

If a PR contains a breaking change and should be merged before the beta release, please mention @parse-community/server-maintenance and we will coordinate with you to merge the PR.

Thanks for your contribution and support during this transition to Parse Server release automation!

@dplewis
Copy link
Member

dplewis commented Jul 17, 2024

Closing via #7304 (comment)

@dplewis dplewis closed this Jul 17, 2024
@olleolleolle olleolleolle deleted the package-json-use-node-10 branch July 18, 2024 06:33
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.

3 participants