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

SDL output from rover graph output has inconsistent leading whitespace on multiline comments #884

Closed
chrskrchr opened this issue Oct 20, 2021 · 11 comments · Fixed by #919 or #931
Closed
Labels

Comments

@chrskrchr
Copy link

chrskrchr commented Oct 20, 2021

Description

When using rover graph introspect to introspect and print the schema SDL to stdout, multiline comments on nested fields are printed with inconsistent leading whitespace. The first line of the multiline comment has two leading spaces as expected, but the subsequent lines are printed with no leading whitespace. e.g.,:

type Book {
  """
  multiline
Book.dummy
description
  """
  dummy: String
}

Steps to reproduce

  1. Clone this repo: https://github.com/chrskrchr/apollo-rover-cli-whitespace
  2. Run npm install
  3. Run node printSchema.js to print the schema using the graphql package's printSchema() function, which will produce output like the following (which includes the expected leading whitespace on multiline comments):
$ node printSchema.js
type Book {
  """
  multiline
  Book.dummy
  description
  """
  dummy: String
}

"""
multiline
Query
description
"""
type Query {
  books: [Book]
}
  1. Run node server.js to start the local ApolloServer
  2. Run npx -p @apollo/[email protected] rover graph introspect http://localhost:4000 to print the schema using the Rover CLI, which will produce output like the following:
$ npx -p @apollo/[email protected] rover graph introspect http://localhost:4000                          
Introspection Response: 

"""Exposes a URL that specifies the behaviour of this scalar."""
directive @specifiedBy on SCALAR
type Book {
  """
  multiline
Book.dummy
description
  """
  dummy: String
}
"""
multiline
Query
description
"""
type Query {
  books: [Book]
}

Note the inconsistent leading whitespace in the multiline comment on the Book.dummy field.

Expected result

All lines in a multiline comment should be printed with the same amount of leading whitespace.

Actual result

The first line of a multiline comment is printed with the expected whitespace, but all subsequent lines are printed with no leading whitespace.

Environment

npx -p @apollo/[email protected] rover info  
Rover Info:
Version: 0.3.0
Install Location: /Users/chris.karcher/.npm/_npx/71693/lib/node_modules/@apollo/rover/node_modules/binary-install/bin/rover
OS: Mac OS 11.6.0 [64-bit]
Shell: /bin/zsh
@chrskrchr chrskrchr added bug 🐞 triage issues and PRs that need to be triaged labels Oct 20, 2021
@chrskrchr
Copy link
Author

chrskrchr commented Oct 20, 2021

Some additional context - this whitespace inconsistency may not seem like a huge deal, but it's generating a bunch of unexpected schema diffs when checking a schema that was produced via printSchema() as described in the PR description against a schema that was published via rover graph introspect | rover graph publish ... --schema -.

@lrlna
Copy link
Member

lrlna commented Oct 22, 2021

Ahhh yea that definitely seems like a bug @chrskrchr ! Thanks for the report. This time around it will take me a bit longer to get to it. I'll aim to get you a fix early next week.

@chrskrchr
Copy link
Author

I'll aim to get you a fix early next week.

Thanks for the heads up! Is there a way for us to consume the fix prior to the official 0.4.0 release? (short of downloading and compiling our own binary)

@chrskrchr
Copy link
Author

@lrlna - do you have an ETA on a fix for this issue?

@lrlna
Copy link
Member

lrlna commented Nov 15, 2021

@chrskrchr so sorry for the delay on this! I had a family emergency and was not able to attend to any of the github issues for a few weeks. We've now merged the fix into main, however.

To answer your earlier question, the easiest way to run rover as it is currently on main is still to build it locally. Once you have it cloned you should be able to run cargo run --release -- install.

@chrskrchr
Copy link
Author

Thanks for the update! Hope that all is well on the home front.

I pulled the rover repo down locally and confirmed with a local build that with this latest fix, we're no longer seeing any unexpected whitespace diffs when running rover graph introspect. 🙌

@chrskrchr
Copy link
Author

@lrlna - do you have an ETA for the 0.4.1 release?

https://github.com/apollographql/rover/milestone/24?closed=1

I tried cross-compiling the Rover CLI for the architecture where we plan to run it (--target=x86_64-unknown-linux-gnu, but ran into issues getting it to build successfully on OS X. I can continue troubleshooting the build issues on my end if needed, but am more than happy to just wait for an official 0.4.1 release if it's not too far out.

@lrlna
Copy link
Member

lrlna commented Nov 16, 2021

@EverlastingBugstopper would be the best person to ask about release dates. Avery, is there a release planned some time soon?

@EverlastingBugstopper
Copy link
Contributor

Planning on cutting a release this week 😊

@lrlna
Copy link
Member

lrlna commented Nov 18, 2021

There is a 0.4.1 release for you @chrskrchr! Let us know how it works out.

@chrskrchr
Copy link
Author

The 0.4.1 release works like a charm - no unexpected diffs at all when using the Rover CLI to introspect and publish schemas. Thanks!

@abernix abernix removed the triage issues and PRs that need to be triaged label Jun 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants