fix: update graphql dependencies to work with Parse Dashboard #7658

FransGH · 2021-10-27T11:57:37Z

Fix for #7650

New Pull Request Checklist

I am not disclosing a vulnerability.
I am creating this PR in reference to an issue.

Issue Description

Parse Server 4.10.4 installs multiple graphql modules
Related issue: #7650

Approach

Updated dependencies in package.json with smallest possible to change to resolve the issue:
"graphql": "15.5.0", // from "15.4.0"
"graphql-relay": "0.7.0", // from "0.6.0"

TODOs before merging

A changelog entry is created automatically using the pull request title (do not manually add a changelog entry)

parse-github-assistant · 2021-10-27T11:57:44Z

Thanks for opening this pull request!

❌ Please edit your post and use the provided template when creating a new pull request. This helps everyone to understand your post better and asks for essential information to quicker review the pull request.

mtrezza · 2021-10-27T13:49:59Z

First, congratulations on your first PR! It seems that you only changed the version numbers in package.json. That is not enough as this does not update the package-lock.json, see the build error:

npm ERR! cipm can only install packages when your package.json and package-lock.json or npm-shrinkwrap.json are in sync. Please update your lock file with npm install before continuing.

To update the packages, revert any changes and run the following:

npm i -E graphql@15.5.0 graphql-relay@0.7.0

Note that dashboard is already on graphql@15.5.1 I think, so you may want to use that version.

Once you made the changes, you can run the tests locally with npm test to see whether all tests pass or other changes are needed. Then we will run the test here with many different Node and MongoDB versions as a final check. Hope this gives some guidance and let me now if you have any questions.

FransGH · 2021-10-27T14:23:43Z

Still need to run the test myself as they fail to run on my dev machine with macOS. Pulling my fork/branch now on a clean docker image..

FransGH · 2021-10-27T15:06:44Z

Did a fresh pull / npm ci of "release-4.x.x" within a ubuntu@latest docker, getting the below error on npm test.
(same problem on my branch, on the 'release' branch I get a different error: can't find module <...>/SchemaCache)

Do I need to install other dependencies to run the tests?

root@d14a71f45616:/parse-server# npm test

> parse-server@4.10.4 pretest /parse-server
> cross-env MONGODB_VERSION=${MONGODB_VERSION:=4.0.4} MONGODB_TOPOLOGY=${MONGODB_TOPOLOGY:=standalone} MONGODB_STORAGE_ENGINE=${MONGODB_STORAGE_ENGINE:=mmapv1} mongodb-runner start

  ◜ Starting a MongoDB deployment to test against...
> parse-server@4.10.4 test /parse-server
> npm run testonly


> parse-server@4.10.4 testonly /parse-server
> cross-env MONGODB_VERSION=${MONGODB_VERSION:=4.0.4} MONGODB_TOPOLOGY=${MONGODB_TOPOLOGY:=standalone} MONGODB_STORAGE_ENGINE=${MONGODB_STORAGE_ENGINE:=mmapv1} TESTING=1 jasmine

/parse-server/spec/ParseLiveQuery.spec.js:952
      expect(prev?.sessionToken).toBeUndefined();
                  ^

SyntaxError: Unexpected token .
    at Module._compile (internal/modules/cjs/loader.js:723:23)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:789:10)
    at Module.load (internal/modules/cjs/loader.js:653:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:593:12)
    at Function.Module._load (internal/modules/cjs/loader.js:585:3)
    at Module.require (internal/modules/cjs/loader.js:692:17)
    at require (internal/modules/cjs/helpers.js:25:18)
    at /parse-server/node_modules/jasmine/lib/jasmine.js:89:5
    at Array.forEach (<anonymous>)
    at Jasmine.loadSpecs (/parse-server/node_modules/jasmine/lib/jasmine.js:88:18)
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! parse-server@4.10.4 testonly: `cross-env MONGODB_VERSION=${MONGODB_VERSION:=4.0.4} MONGODB_TOPOLOGY=${MONGODB_TOPOLOGY:=standalone} MONGODB_STORAGE_ENGINE=${MONGODB_STORAGE_ENGINE:=mmapv1} TESTING=1 jasmine`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the parse-server@4.10.4 testonly script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /root/.npm/_logs/2021-10-27T14_40_21_215Z-debug.log
npm ERR! Test failed.  See above for more details.
root@d14a71f45616:/parse-server#

mtrezza · 2021-10-27T16:10:23Z

Which node version are you using?

FransGH · 2021-10-27T18:05:21Z

I've tried again with the same node/npm versions as in the workflow:

root@d14a71f45616:/parse-server# npm -v
6.14.12
root@d14a71f45616:/parse-server# node -v
v10.24.1

When I do npm run coverage on either on "release-4.x.x" or "tags/4.10.4" it always fails on

/parse-server/spec/ParseLiveQuery.spec.js:952
      expect(prev?.sessionToken).toBeUndefined();

I'll try to troubleshoot this a bit further. Guess it must be some dependency or could it be that the commits I'm using to test have an issue?

mtrezza · 2021-10-27T20:25:18Z

Can you try to use a higher node version? I suspect optional chaining (optional?.key) does not exist in Node 10. The tests are not babel compiled, so if a test uses an unsupported syntax, it won't run.

mtrezza · 2021-10-27T20:50:17Z

You can run the test in Node 10, but change test to not use optional chaining.

package.json

mtrezza · 2021-10-27T22:49:40Z

The test fail, luckily the test logs output on which line. I suggest running this locally for debugging to see what exactly causes the issue at that line:

Error: Object not found.
at then.then.result (/home/runner/work/parse-server/parse-server/lib/Controllers/DatabaseController.js:49:3084)
at process._tickCallback (internal/process/next_tick.js:68:7)

It's the same error as in #7599. That means the CI on that branch is currently broken. I opened #7659 for this. If you find a solution to this, we can open a separate PR for a fix. You could just make a new branch on 4.x without any changes and try to run the tests locally on Node 10 to investigate. We can then follow up the discussion there. After that fix, this PR can be merged.

FransGH · 2021-10-27T23:02:17Z

I left that one commit from 7599 out as I was unclear what it did and like to keep the PR to an absolute minimum. The other two commits were just syntax issues.

On the "object not found" output: this is not an error but seem to be 'expected' trace output from EnableExpressErrorHandler.spec.js. I see this too when running the test locally and it's not an issue.

Don't know at the moment why the tests again fail, I'll have a closer look...

FransGH · 2021-10-27T23:37:39Z

There seem to be 15min timeouts within the latest CI workflows.

My guess is that GitHub Actions always triggers the latest revision of the workflow file, causing ci.yml from the alpha branch to precede over the release-4.x.x branch.

Could you maybe increase the 'matrix' timeouts to 30min in the alpha branch and run the workflows again?
They should than either run trough or get killed after 30min...

mtrezza · 2021-10-27T23:41:14Z

The workflow that runs in the one that is in the brach that triggered the workflow, so the workflow file is correct. They did not time out, I stopped the CI because it stalled. A test should not take longer than 15 mins, more like <10 mins.

If you run npm test locally on npm 10, do they all pass?

FransGH · 2021-10-28T00:40:53Z

I also canceled after a longer time before doing the last commit. It didn't look stalled and was just taking a very long to complete. Will now let it run overnight...

What I don't understand is how 4.10.4 got released when the ci workflow seems to be broke?

It looks like you are using a different CI workflow in your fork? Since this seems to work, wouldn't it make sense to merge that into into a 4.10.x PR?

FransGH · 2021-10-28T13:11:50Z

Ok, found the failed test, it's related to this GHSA-7pr3-p5fm-8r9x.

The new test "should strip out session token in LiveQuery" fails and is included in both ParseLiveQuery.spec.js and ParseUser.spec.js. When I comment it out, the test complete ok on node 10. I'll try to fix this test and add it to the PR...

mtrezza · 2021-10-28T14:19:01Z

Amazing! Can you please open a separate PR for the fix and reference issue #7659?

FransGH · 2021-10-28T14:43:44Z

@mtrezza, sorry, wanted to create a new branch/pr for this but mixed something up. Maybe just let the workflows run to see if the issue is really fixed? Happy to create a separate PR for this if the CI issue is solved..

mtrezza · 2021-10-28T21:28:15Z

Workflows running

codecov · 2021-10-28T21:34:34Z

Codecov Report

❗ No coverage uploaded for pull request base (release-4.x.x@b465f7b). Click here to learn what that means.
The diff coverage is n/a.

@@               Coverage Diff                @@
##             release-4.x.x    #7658   +/-   ##
================================================
  Coverage                 ?   93.83%           
================================================
  Files                    ?      169           
  Lines                    ?    12442           
  Branches                 ?        0           
================================================
  Hits                     ?    11675           
  Misses                   ?      767           
  Partials                 ?        0

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 b465f7b...eb04ae9. Read the comment docs.

mtrezza · 2021-10-28T21:43:19Z

The CI has passed, well done! Could you extract the CI fix into a separate PR so we can evaluate that separately?

FransGH · 2021-10-28T23:06:05Z

New PR #7661 for CI fix created. What do we do with this one?

mtrezza · 2021-10-28T23:58:49Z

This one is for the graphql upgrade as I understand? So let's first merge the CI fix, then we can merge this one.

mtrezza · 2021-10-30T17:24:07Z

@FransGH Could you merge the branch into this PR, so we can see whether the CI passes? #7661 has been merged.

mtrezza · 2021-10-30T21:10:06Z

There seems to be a mix up with the other PR that optimizes the CI time? Could you reduce this PR to only upgrade the GraphQL dependencies?

FransGH · 2021-10-30T22:21:54Z

Sorry for that, fix it!

package.json

… versions

FransGH · 2021-10-30T23:15:16Z

Ok, 1st time working from a fork, doing a PR and discovering 'fetch upstream'.. Now things should be ok again :)

mtrezza · 2021-10-31T12:49:05Z

You may find "GitHub Desktop" helpful that gives you a visual UI for the usual operations needed for contributing.

The next dashboard release will use "graphql": "15.6.1" while this PR upgrades to "graphql": "15.5.1". Will that cause the same error, or was the error only because of a major version difference? If the graphql versions have to be completely identical, then we may have to open an issue with the maintainers of the package that is throwing that error.

mtrezza · 2021-11-01T15:15:59Z

Note: before merging this; auto-release needs to be configured for the release-4.x.x branch.
#7674

mtrezza · 2022-02-12T15:46:13Z

Auto-release has been added to release-4.x.x branch, so this can be merged if it passes review.

## [4.10.6](4.10.5...4.10.6) (2022-02-12) ### Bug Fixes * update graphql dependencies to work with Parse Dashboard ([#7658](#7658)) ([350ecde](350ecde))

parseplatformorg · 2022-02-12T16:08:24Z

🎉 This change has been released in version 4.10.6

parse-github-assistant · 2022-02-12T16:08:28Z

The label state:released-4.x.x cannot be used here.

FransGH changed the title ~~Fix graphql dependencies for 4.10.x~~ fix: update graphql dependencies for 4.10.x Oct 27, 2021

FransGH mentioned this pull request Oct 27, 2021

Parse Server 4.10.4 installs multiple graphql modules #7650

Closed

FransGH mentioned this pull request Oct 27, 2021

test: fix failing tests after removal of session token #7599

Merged

2 tasks

mtrezza reviewed Oct 27, 2021

View reviewed changes

package.json Outdated Show resolved Hide resolved

FransGH force-pushed the fix-graphql-deps branch from 974e903 to 703dbd4 Compare October 30, 2021 22:07

mtrezza reviewed Oct 30, 2021

View reviewed changes

package.json Outdated Show resolved Hide resolved

reset branch to latest release-4.x.x and bumped graphql/graphql-relay…

7199fc5

… versions

FransGH force-pushed the fix-graphql-deps branch from 7c52323 to 7199fc5 Compare October 30, 2021 23:10

Merge branch 'release-4.x.x' into fix-graphql-deps

eb04ae9

mtrezza changed the title ~~fix: update graphql dependencies for 4.10.x~~ fix: update graphql dependencies for Parse Server 4.x to work with Parse Dashboard Feb 12, 2022

mtrezza changed the title ~~fix: update graphql dependencies for Parse Server 4.x to work with Parse Dashboard~~ fix: update graphql dependencies to work with Parse Dashboard Feb 12, 2022

mtrezza approved these changes Feb 12, 2022

View reviewed changes

mtrezza merged commit 350ecde into parse-community:release-4.x.x Feb 12, 2022

parseplatformorg pushed a commit that referenced this pull request Feb 12, 2022

chore(release): 4.10.6 [skip ci]

cd41626

## [4.10.6](4.10.5...4.10.6) (2022-02-12) ### Bug Fixes * update graphql dependencies to work with Parse Dashboard ([#7658](#7658)) ([350ecde](350ecde))

parseplatformorg added the state:released-4.x.x Released as LTS version label Feb 12, 2022

parse-github-assistant bot removed the state:released-4.x.x Released as LTS version label Feb 12, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: update graphql dependencies to work with Parse Dashboard #7658

fix: update graphql dependencies to work with Parse Dashboard #7658

FransGH commented Oct 27, 2021 •

edited by mtrezza

Loading

parse-github-assistant bot commented Oct 27, 2021 •

edited

Loading

mtrezza commented Oct 27, 2021 •

edited

Loading

FransGH commented Oct 27, 2021

FransGH commented Oct 27, 2021

mtrezza commented Oct 27, 2021

FransGH commented Oct 27, 2021

mtrezza commented Oct 27, 2021 •

edited

Loading

mtrezza commented Oct 27, 2021

mtrezza commented Oct 27, 2021 •

edited

Loading

FransGH commented Oct 27, 2021 •

edited

Loading

FransGH commented Oct 27, 2021

mtrezza commented Oct 27, 2021 •

edited

Loading

FransGH commented Oct 28, 2021

FransGH commented Oct 28, 2021

mtrezza commented Oct 28, 2021 •

edited

Loading

FransGH commented Oct 28, 2021 •

edited

Loading

mtrezza commented Oct 28, 2021

codecov bot commented Oct 28, 2021 •

edited

Loading

mtrezza commented Oct 28, 2021 •

edited

Loading

FransGH commented Oct 28, 2021

mtrezza commented Oct 28, 2021 •

edited

Loading

mtrezza commented Oct 30, 2021 •

edited

Loading

mtrezza commented Oct 30, 2021 •

edited

Loading

FransGH commented Oct 30, 2021 •

edited

Loading

FransGH commented Oct 30, 2021

mtrezza commented Oct 31, 2021 •

edited

Loading

mtrezza commented Nov 1, 2021 •

edited

Loading

mtrezza commented Feb 12, 2022 •

edited

Loading

parseplatformorg commented Feb 12, 2022

parse-github-assistant bot commented Feb 12, 2022

fix: update graphql dependencies to work with Parse Dashboard #7658

fix: update graphql dependencies to work with Parse Dashboard #7658

Conversation

FransGH commented Oct 27, 2021 • edited by mtrezza Loading

New Pull Request Checklist

Issue Description

Approach

TODOs before merging

parse-github-assistant bot commented Oct 27, 2021 • edited Loading

Thanks for opening this pull request!

mtrezza commented Oct 27, 2021 • edited Loading

FransGH commented Oct 27, 2021

FransGH commented Oct 27, 2021

mtrezza commented Oct 27, 2021

FransGH commented Oct 27, 2021

mtrezza commented Oct 27, 2021 • edited Loading

mtrezza commented Oct 27, 2021

mtrezza commented Oct 27, 2021 • edited Loading

FransGH commented Oct 27, 2021 • edited Loading

FransGH commented Oct 27, 2021

mtrezza commented Oct 27, 2021 • edited Loading

FransGH commented Oct 28, 2021

FransGH commented Oct 28, 2021

mtrezza commented Oct 28, 2021 • edited Loading

FransGH commented Oct 28, 2021 • edited Loading

mtrezza commented Oct 28, 2021

codecov bot commented Oct 28, 2021 • edited Loading

Codecov Report

mtrezza commented Oct 28, 2021 • edited Loading

FransGH commented Oct 28, 2021

mtrezza commented Oct 28, 2021 • edited Loading

mtrezza commented Oct 30, 2021 • edited Loading

mtrezza commented Oct 30, 2021 • edited Loading

FransGH commented Oct 30, 2021 • edited Loading

FransGH commented Oct 30, 2021

mtrezza commented Oct 31, 2021 • edited Loading

mtrezza commented Nov 1, 2021 • edited Loading

mtrezza commented Feb 12, 2022 • edited Loading

parseplatformorg commented Feb 12, 2022

parse-github-assistant bot commented Feb 12, 2022

FransGH commented Oct 27, 2021 •

edited by mtrezza

Loading

parse-github-assistant bot commented Oct 27, 2021 •

edited

Loading

mtrezza commented Oct 27, 2021 •

edited

Loading

mtrezza commented Oct 27, 2021 •

edited

Loading

mtrezza commented Oct 27, 2021 •

edited

Loading

FransGH commented Oct 27, 2021 •

edited

Loading

mtrezza commented Oct 27, 2021 •

edited

Loading

mtrezza commented Oct 28, 2021 •

edited

Loading

FransGH commented Oct 28, 2021 •

edited

Loading

codecov bot commented Oct 28, 2021 •

edited

Loading

mtrezza commented Oct 28, 2021 •

edited

Loading

mtrezza commented Oct 28, 2021 •

edited

Loading

mtrezza commented Oct 30, 2021 •

edited

Loading

mtrezza commented Oct 30, 2021 •

edited

Loading

FransGH commented Oct 30, 2021 •

edited

Loading

mtrezza commented Oct 31, 2021 •

edited

Loading

mtrezza commented Nov 1, 2021 •

edited

Loading

mtrezza commented Feb 12, 2022 •

edited

Loading