Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure this works for RegExp subclasses #11

Open
ljharb opened this issue Aug 12, 2015 · 8 comments
Open

Ensure this works for RegExp subclasses #11

ljharb opened this issue Aug 12, 2015 · 8 comments

Comments

@ljharb
Copy link
Collaborator

ljharb commented Aug 12, 2015

I don't see spec text yet, so it may not be a problem yet - but let's make sure that

class Foo extends RegExp {} ; Foo.make`${foo}`

returns an instanceof Foo, not just RegExp.

@mikesamuel
Copy link
Owner

So we would use the this value as the constructor?

Would it be preferable to invoke the constructor assuming it takes (source, flags) or to try to do something Object.create based and then call the RegExp constructor to initialize if possible?

@bergus
Copy link
Collaborator

bergus commented Aug 12, 2015

Just use Reflect.construct. I think it's safe to assume it takes (source, flags). Using RegExpInitialize directly should be avoided.

@ljharb
Copy link
Collaborator Author

ljharb commented Aug 12, 2015

It's the same logic as Array.from/Array.of imo - it uses the this value for the species-or-constructor.

Why would I want to create a RegExp and then use Reflect.construct in order to get a subclass of RegExp, when I only want the subclass already? Wouldn't that call RegExpInitialize twice when I only need it once?

@bergus
Copy link
Collaborator

bergus commented Aug 12, 2015

Yeah, just like Array.of/Array.from/Promise.resolve - not using @@species though, that's for instance methods.

So new this(pattern, flags) (or Reflect.construct(this, [pattern, flags])); maybe with isConstructor(this) ? this : RegExp.
My hint at Reflect was unnecessary, I mistakenly thought that we could use it for variadic arguments - which are not needed here.

@ljharb
Copy link
Collaborator Author

ljharb commented Aug 12, 2015

yeah basically new this in the implementation of make should work fine, presuming that it includes a spec step that throws if it doesn't have a [[RegExpMatcher]] internal slot.

@bergus
Copy link
Collaborator

bergus commented Aug 12, 2015

Nah, there is no [[RegExpMatcher]] slot on regex constructor functions, it's an instance slot.
I don't think it's possible to detect whether a function is an ES6 subclass that extends RegExp and calls super to it, but I don't think it's necessary either. There's nothing wrong with being lax about what the constructor does - it's not our business.
The closest you could get afaik is RegExp == this || RegExp.isPrototypeOf(this), but even that can be fooled.

@ljharb
Copy link
Collaborator Author

ljharb commented Aug 12, 2015

ah, good call

@mikesamuel
Copy link
Owner

Pull request: #18

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants