Change return value of feedback.warning when there is no warning given about zxcvbn HOT 8 CLOSED

That use case seems more than reasonable 👍

Will change it to null, which would be a breaking change but good thing i'm currently developing against a new major version anyway

from zxcvbn.

I don't really understand this.
An empty string is a falsy value so if(feedback.warning) should work fine 🤔
Additional there will always be only one warning because this is something set by the highest matcher and are not combined same with the suggestions.

from zxcvbn.

An empty string is a falsy value so if(feedback.warning) should work fine

While it does work, it relies on JavaScripts coercing, which is simply not clean, at least in my view.
Image that you forget to check with an if, then it will just proceed with that empty string. If you have null instead, the TypeScript compiler will complain.

from zxcvbn.

I found this library yesterday and added it to my code. I already have other stuff there which is why I merge the results (code simplified):

type CheckPasswordResult = {
  warnings: string[]
};

function examplePasswordCheck(password: string): CheckPasswordResult {
  const zxcvbnResult = zxcvbn(password);
  const someOtherResult = someOtherStuff(password);

  const warnings: string[] = [...someOtherResult.warnings, zxcvbnResult.feedback.warning];
  return { warnings };
}

function consumer(): void {
  const passwordResult = examplePasswordCheck('password-from-request');

  if (passwordResult.warnings.length > 0) {
    throw new Error('Bad password.');
  }

  doSomethingWithThePassword();
}

As I always work with null/undefined instead of an empty string, this was what I did intuitively (no check for an empty string). There will be no compiler error from TypeScript. So I run the code... and what I get is an unwanted runtime behavior, even though I check the length of the warnings array to see if there are warnings. Now I have to take some time and debug what happens.

Imagine it would return null instead of ''. TypeScript would complain that null is not allowed inside of that string[] and I could see instantly what the problem is.

With just string it is ambiguous, while string | null (or string | undefined) is unambiguous/explicit.
For your use-case nothing changes. You can still use if (feedback.warning) and TS will give you a string inside that if-block.
You can even get rid of that null if you really want by using strictNullChecks: false.

from zxcvbn.

Apart from this change, it will be everything since the MR #174
Which means:

• language packages treeshakeable
• removal of default exports
• networkError handler options for the pwned matcher
• diceware dictionary with separated scoring for the dictionary matcher
• a compound splitter which is used by the generator of the dictionaries => smaller bundle size for the spanish package

Additionally i'm trying to work on a "unmunger" instead of the l33t speak to search for more complex l33t speak stuff. If i don't get this done i can at least fix #118 which is kind of breaking.

Another thing that i'm working on is trying to fix the ReDOS vulnerability dropbox/zxcvbn#327

It would be nice if i could land the seperator MR too #115

from zxcvbn.

@MrWook I have a new PR for implementing an "unmunger" if you want to look through it: GoSimpleLLC/nbvcxz#84

I haven't had a chance to look through it to merge yet, but wanted to bring it to your attention.

from zxcvbn.

It was @U-4-E-A improved example with if(feedback.warning), which is ignoring an empty warning just fine.

What is the use case where a typescript compile will throw an error?

I will check with a few colleagues about "clean code" to get some more opinions on this matter.

from zxcvbn.

Victory!

Can I ask what changes we will see in the new major version?

from zxcvbn.