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: prettier issue #411

Merged
merged 1 commit into from
Nov 26, 2024

Conversation

SujalXplores
Copy link

This pull request includes a minor change to the app/utils/logger.ts file. The change simplifies the initialization of the currentLevel variable by removing unnecessary parentheses.

Before:

image

After:

image

  • app/utils/logger.ts: Simplified the initialization of the currentLevel variable by removing unnecessary parentheses.

@SujalXplores
Copy link
Author

@wonderwhy-er, could you please review this PR?

@wonderwhy-er
Copy link
Collaborator

I am not sure this is right, does it not change order of what is happening?

@SujalXplores
Copy link
Author

I am not sure this is right, does it not change order of what is happening?

This is actually a prettier/prettier rule error, not a logical code issue!

image

Prettier has an opinion about unnecessary parentheses around nullish coalescing operations (??). Since ?? has its own operator precedence that's well defined, Prettier considers the parentheses redundant in this case.

The operator precedence is:

  • ?? has lower precedence than ?: (ternary)
  • So a ?? b ? c : d is automatically interpreted as (a ?? b) ? c : d

Therefore, both versions are actually functionally identical:

// Both of these do exactly the same thing:
(import.meta.env.VITE_LOG_LEVEL ?? import.meta.env.DEV) ? 'debug' : 'info'
import.meta.env.VITE_LOG_LEVEL ?? import.meta.env.DEV ? 'debug' : 'info'

We can safely remove the parentheses to satisfy Prettier - this is purely a style preference by Prettier and not a logical change to the code.

Here's a simple test code you can paste directly into Chrome's console to verify the operator precedence:

// Let's simulate both env variables
const env = {
  VITE_LOG_LEVEL: undefined,  // or try with null, or "debug"
  DEV: true                   // try with false too
};

// Version 1 (with parentheses)
const result1 = (env.VITE_LOG_LEVEL ?? env.DEV) ? 'debug' : 'info';
console.log('With parentheses:', result1);

// Version 2 (without parentheses)
const result2 = env.VITE_LOG_LEVEL ?? env.DEV ? 'debug' : 'info';
console.log('Without parentheses:', result2);

// You can also try different values:
console.log('Test cases:');
console.log('undefined ?? true →', undefined ?? true);
console.log('undefined ?? false →', undefined ?? false);
console.log('"debug" ?? true →', "debug" ?? true);
console.log('null ?? true →', null ?? true);

Would you like me to try any specific test case?

@wonderwhy-er
Copy link
Collaborator

You are right, thanks for sharing example.

@wonderwhy-er wonderwhy-er merged commit 60cfb5a into stackblitz-labs:main Nov 26, 2024
1 check passed
@SujalXplores SujalXplores deleted the fix/prettier-issue branch November 26, 2024 07:56
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.

2 participants