fix: prevent error on not set location href#12494
Conversation
|
|
| typeof location !== 'undefined' | ||
| ) { | ||
| const prefix = location.href.replace(/\/$/, '') | ||
| const prefix = location.href?.replace(/\/$/, '') |
There was a problem hiding this comment.
I think we could replace 1546 with typeof location?.href === 'string'. If href isn't defined, then we don't need to call replace
There was a problem hiding this comment.
@mschakulat you haven't fixed this part yet, which I agree would be a nicer check
There was a problem hiding this comment.
@bluwy Yes, that's correct. I have adjusted it.
There was a problem hiding this comment.
Unfortunately condition must be typeof location !== 'undefined'.
In my project with svelte-kit and scss doesn't work new condition, because location is not defined.
Optional chaining doesn't work on not defined variables.
|
Would you give more info about your environment that is triggering this bug? Interesting that we didn't get a report before. |
Unfortunately i can't say too much about it. We use a proprietary package (https://bitmovin.com/). The code is encrypted and therefore the context is hard to see. But I see that the 'location' object is edited there: |
mschakulat
force-pushed
the
fix/undefined_href_issue
branch
from
be87566 to
e2ab663
Compare
mschakulat
requested review from
bluwy and
patak-cat
and removed request for
bluwy and
patak-cat
4 participants
Description
The current implementation of the function
cleanScssBugUrlchecks if the 'window' and 'location' globals are defined, and then replaces the prefix of the URL with the location href. However, in some cases, the 'location.href' value can be undefined, resulting in an error.To resolve this issue, I've updated the code to use optional chaining (?.) to ensure that the 'replace' function only runs if 'location.href' is defined. This modification ensures that the function can handle cases where 'location.href' is undefined and prevents errors from being thrown.
This change has been tested and verified to work as expected.
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123).