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

chose(deps): update GraphQLCompiler dependencies #181

Merged
merged 13 commits into from Dec 7, 2023

Conversation

calvincestari
Copy link
Member

@calvincestari calvincestari commented Dec 4, 2023

This PR should remain a Draft until there is a resolution to graphql/graphql-js#3927. Until that is resolved the rollup command will fail with the following error:

[!] RollupError: Expected ident (Note that you need plugins to import files that are not JavaScript)
node_modules/graphql/jsutils/instanceOf.mjs (11:43)

I've downgraded the graphql-js dependency to 17.0.0-alpha.2 to fix the above error. Once there is a resolution to graphql/graphql-js#3927 we can update the graphql-js dependency to the latest available version.

Combines the changes from PRs:

Notes:

  • There are also some manual dependency changes required to get compatibility with some of the other dependencies.
  • The auto rollup script has been changed to fail on error (set -e) which will prevent partial updates to the Swift file which might be perceived as valid changes. I also applied some changes from a Bash script validator.
  • Includes a new CI job to validate that the bundle can be built, rollup will succeed and there are no missed changes that should have been committed to the Swift file.

@calvincestari calvincestari force-pushed the deps/secops/graphqlcompiler-2023-12-04 branch from 5d16af5 to 0345195 Compare December 7, 2023 07:02
@calvincestari calvincestari marked this pull request as ready for review December 7, 2023 07:15

SCRIPT_DIR=$( cd -- "$( dirname -- "${BASH_SOURCE[0]}" )" &> /dev/null && pwd )
output_file="$SCRIPT_DIR/../ApolloCodegenFrontendBundle.swift"
$( cd "$SCRIPT_DIR" && rollup -c )
cd "$SCRIPT_DIR" && npm run build
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use the package version of rollup instead of needing a global installation of it.

"test": "jest"
},
"engines": {
"npm": ">=7"
},
"dependencies": {
"graphql": "17.0.0-alpha.3",
"graphql": "17.0.0-alpha.2",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the solution to fixing the Node error in the description. I do not believe we need anything in alpha.3 - all our codegen tests still pass.

@@ -207,6 +207,24 @@ jobs:
shell: bash
run: |
./scripts/run-test-codegen-configurations.sh -t

verify-frontend-bundle-latest:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be more efficient to include this as a step in the codegen unit tests CI job instead of spinning up another job? Not sure if it matters since the CI jobs on these runners are free minutes?

@calvincestari calvincestari merged commit d7d1c4b into main Dec 7, 2023
15 checks passed
@calvincestari calvincestari deleted the deps/secops/graphqlcompiler-2023-12-04 branch December 7, 2023 18:49
BobaFetters pushed a commit to apollographql/apollo-ios-codegen that referenced this pull request Dec 7, 2023
BobaFetters pushed a commit that referenced this pull request Dec 7, 2023
4cc288c4 chose(deps): update GraphQLCompiler dependencies (#181)

git-subtree-dir: apollo-ios-codegen
git-subtree-split: 4cc288c43509675900fa6c7452ab587fe77efdb9
BobaFetters pushed a commit that referenced this pull request Dec 7, 2023
…iler dependencies

git-subtree-dir: apollo-ios-codegen
git-subtree-mainline: ff034f9
git-subtree-split: 4cc288c43509675900fa6c7452ab587fe77efdb9
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants