fix: avoid regex lookbehind for compatibility #7270
Conversation
| @@ -624,12 +624,12 @@ export abstract class QueryBuilder<Entity> { | |||
|
|
|||
| if (replacementKeys.length) { | |||
| statement = statement.replace(new RegExp( | |||
| `(?<=[ =\(]|^.{0})` + | |||
| `([ =\(]|^.{0})` + | |||
There was a problem hiding this comment.
Unrelated to this change but while we're here what is the effect of the .{0}?
There was a problem hiding this comment.
Not sure, seems to have originated here.
There was a problem hiding this comment.
considering the code-change: since this is not reverting the code to the last known working state but "just" removing the lookbehind expression from the regex - can you comment on how this code still accomplishes what it should do?
And maybe we should add a code-comment here about the regex-compatibility issue so that the next guy is not introducing the same thing again.
There was a problem hiding this comment.
The previous code did the same thing with the replacement. Lookahead/behind are just a simplification. I added a comment.
|
@Vluf @chriswep can you guys check if this fix works on react-native? If so, I'll merge this PR. |
|
@pleerock This PR fixes my issue on react-native, thanks! |
@pleerock to clarify, the issue is not with react-native per se, but with a lot of web-environments not supporting regex-lookbehind, including Safari. can't do a test right now, however since the lookbehind is removed it should not throw the error anymore. i can't speak into if the corresponding method still works correctly (see code commit above) - is this method tested properly? |
yes, I got it. My point was to ask people (who use react native or web-env) to test these changes. |
Lookbehind isn't supported in Safari/JavaScriptCore environments. Closes: #7026
|
Thank you @mohd-akram |
Description of change
Closes #7026
Pull-Request Checklist
masterbranchnpm run lintpasses with this changenpm run testpasses with this changeFixes #0000