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

fix: re-export setVerbosity from ts-invariant #11667

Closed
wants to merge 1 commit into from

Conversation

iiroj
Copy link

@iiroj iiroj commented Mar 13, 2024

Hello,

as a happy user of Apollo Client, I would like to completely suppress the invariant log errors in my production environment. I don't want to load all the error codes into the bundle, nor do I want to output the fallback message linking to https://go.apollo.dev/ into the console.

I opened this small PR that adds a new method for achieving this:

import { setVerbosity } from "@apollo/client/dev";

if (!__DEV__) {  // Silence messages when not in a dev environment
  setVerbosity("silent");
}

Where setVerbosity is just directly re-exported from ts-invariant.

There doesn't seem to be any existing tests for this functionality, but I'm happy to create some if necessary. EDIT: I found a place to add an unit test.

Feel free to merge, edit, or reject this pull request. Since the change is small enough, I figured it's easier to just create a PR rather than submit an issue for the feature request first.

@iiroj iiroj requested review from a team as code owners March 13, 2024 10:57
Copy link

netlify bot commented Mar 13, 2024

👷 Deploy request for apollo-client-docs pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit 6677a17

Copy link

changeset-bot bot commented Mar 13, 2024

🦋 Changeset detected

Latest commit: 6677a17

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@apollo/client Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@@ -117,3 +127,11 @@ test("base invariant(false, 6, ...), raises fallback", () => {
)
);
});

test("setVerbosity('silent') disables logging to console", () => {
Copy link
Member

Choose a reason for hiding this comment

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

Seeing this:

I believe what this doesn't mute are actual errors triggered via invariant(false, "errorMessage").

This is essentially necessary, since errors need to interrupt code flow via throw.

It might be worth an additional test to show that, and adding a sentence in the docs that this will silence all kinds of logs, but errors will still be thrown.

Copy link
Author

Choose a reason for hiding this comment

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

Looking at the underlying implementation, you are probably right; so this doesn't actually achieve what I want, which was to completely prevent the invariant fallback error message from appearing in the console (the one linking to go.apollo.dev).

What do you think, maybe I'll just close this issue?

Copy link
Member

Choose a reason for hiding this comment

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

I think it's still a valuable contribution.

As for errors: they won't automatically be logged to the console, you can still catch and handle them (or add a document.addListener('error') to handle errors that might slip through) to catch them before they reach the console. But they still need to be thrown, or you'll have a different code flow than anyone else.

But maybe coming back to your initial motivation:
What is your main motivation to prevent the links from ending up in the console?
We are currently in the process of moving the error page into a separate page, outside of the Apollo Client Documentation.
Would that already address your concerns regarding that link?

Copy link
Author

Choose a reason for hiding this comment

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

I can contribute this MR with some additional tests. I also realised that this verbosity setting probably would affect other usages of ts-invariant as well, so it's worth mentioning.

As for the error page, for me it's a security issue since it's an external domain and something that's difficult to test to verify.

@phryneas
Copy link
Member

phryneas commented Mar 19, 2024

The error message case should be sufficiently covered in #11694.

But you're right in your comment about ts-invariant being used by other libraries.
It's easy to forget since it's one of "our" libraries, but other projects have picked it up as a dependency. (I would make a bet that none of your non-Apollo-Client libraries are using it, though - it's not that widespread 😉 )

So I think it's not the best idea to re-export setVerbosity as an Apollo Client export.

Could you reduce this PR to the docs change?

I'm sorry, but you'll have to add ts-invariant to your dependencies if you're strict about it.
On the other hand, you might not even need it anymore after #11694. 😄

@iiroj iiroj closed this Mar 20, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants