fix regexp without flags throwing SyntaxError

[jakub-g]

It seems that RegExp constructor doesn't like the 2nd param to be null, but it can be undefined

SyntaxError: Invalid flags supplied to RegExp constructor 'null'

[jakub-g]

I also added one commit to cast the regex to string, because regex.exec(regex) looked confusing, I think it makes sense to be more explicit for readability.

[jonschlinkert]

Thanks for the PR!

looked confusing, I think it makes sense to be more explicit for readability.

Understandable. However, they are two different values.

.source is the property that holds the actual source string for the regular expression, whereas .toString() converts the regular expression along with the opening and closing delimiters into a string.

const regex = /foo/;
console.log(regex.source); //=> 'foo'
console.log(regex.toString()); //=> '/foo/' 

I'd prefer that the PR be isolated and focused on the problem being solved. Would it be too much trouble for you to revert the change to .source?

[jakub-g]

Hello,

I think you misread the PR by looking at it as a whole.

There are two separate commits here:

• the first is adding || undefined to avoid potential null (and extracting a variable)
• the second is changing /\w+$/.exec(val) to /\w+$/.exec(val.toString()) which is equivalent according to exec spec, but more explicit (https://tc39.github.io/ecma262/#sec-regexp.prototype.exec states in step 4 that the parameter is cast to string before execution).

I can open separate PRs if you want but I thought that 2 commits in one PR was sufficient :)

[jonschlinkert]

I think you misread the PR by looking at it as a whole.

Yes, .toString() does return a string, but not the desired string. You are misinterpreting the spec and my previous comment. In particular, note the following:

See the slashes?

Here is what happens when you use the value returned by .toString():

// the regular expression is incorrect, as we do not want the leading and trailing slashes added to the string.
console.log(new RegExp(regex.toString())) //=> /\/foo\//

Again, do you see the slashes? We don't want the slashes. .source is the correct, and is the only value we can to use here.

looked confusing, I think it makes sense to be more explicit for readability.

1. Your confusion doesn't make .source wrong. It means that you're not familar with the RegExp API.
2. Using the .toString() function is by no means more explicit. .toString() may be used on any JavaScript object, including RegExp, to return a string representing the object, but .source is used to explicitly return the actual source string of a compiled regular expression. That is not the same thing as it's string representation.
3. Even if all other factors were equal, your point that .toString() is more "readable" than .source seems flawed, given that:

• .source is 4 characters shorter
• .source is all lower case
• .source is a simple property (accessor), whereas .toString() is a method that needs to be called

[jakub-g]

Once again, I didn't change .source to .toString(), I changed val to val.toString(), where val was RegExp while .exec expects a String.
(The tests were green after the change, are there any more tests to be added maybe?)

But anyway, I dropped that commit, since it's not important -- can you check if it's ok now?

[estrattonbailey]

@jonschlinkert running into the SyntaxError as well using clone-deep, FWIW.