feat: add fallback if custom getLocalIdent returns null #1193
Conversation
Fallback to default `getLocalIdent` function if the `getLocalIdent` function provided in the module options returns a `null`/`undefined`.
|
|
Codecov Report
@@ Coverage Diff @@
## master #1193 +/- ##
==========================================
+ Coverage 99.24% 99.40% +0.15%
==========================================
Files 10 10
Lines 666 670 +4
Branches 212 213 +1
==========================================
+ Hits 661 666 +5
+ Misses 5 4 -1
Continue to review full report at Codecov.
|
src/utils.js
Outdated
|
|
||
| // A null/undefined value signals that we should invoke the default | ||
| // getLocalIdent method. | ||
| if (localIdent == null && getLocalIdent !== defaultGetLocalIdent) { |
There was a problem hiding this comment.
Why we need getLocalIdent !== defaultGetLocalIdent?
There was a problem hiding this comment.
In the situation where there wasn't a custom getLocalIdent function defined and the default function returned null or undefined for some reason, I didn't want to run the default again. It wouldn't hurt anything to do so, but it also wouldn't help anything either! ;)
There was a problem hiding this comment.
I think it is bad logic, we don't need getLocalIdent !== defaultGetLocalIdent, you can't use defaultGetLocalIdent in own code
There was a problem hiding this comment.
Well, at line 140 in the utils.js, the getLocalIdent property defaults to defaultGetLocalIdent function. So if a user doesn't provide a function, they are implicitly using the default function. The intent for the !== defaultGetLocalIdent check is to handle the situation where the user didn't provide a getLocalIdent function and defaultGetLocalIdent returned a null or undefined.
That may never happen, as there may already be safeguards in place to keep that from happening - and even if it did run the same function twice, it probably wouldn't be a big deal - so that check can be removed.
README.md
Outdated
| @@ -848,6 +848,8 @@ Default: `undefined` | |||
|
|
|||
| Allows to specify a function to generate the classname. | |||
| By default we use built-in function to generate a classname. | |||
| If the custom function returns `null` or `undefined`, we fall back to the | |||
tvsbrent
force-pushed
the
issue-1191-get-local-ident-fallback
branch
from
b406380
to
53d88c1
Compare
|
Sorry, I had to fix a commit subject to correct the lint error. |
|
Thanks! |
This PR contains a:
Motivation / Use-Case
With the release of the 4.x version of the css-loader, the behavior that allowed a
getLocalIdentfunction to return a falsy value to fallback to the defaultgetLocalIdentfunction was lost. That original behavior was introduced in this pull request.This PR reintroduces that behavior, but instead requires the function to return
nullorundefined.Breaking Changes
n/a
Additional Info