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

ThisExpression as a property #45

Open
eliekhoury opened this issue Jun 20, 2016 · 4 comments
Open

ThisExpression as a property #45

eliekhoury opened this issue Jun 20, 2016 · 4 comments
Labels

Comments

@eliekhoury
Copy link

Shouldn't this in test.this be considered as an Identifier instead of a ThisExpression?

@LeaVerou
Copy link
Collaborator

@EricSmekens Actually, what's the point of ThisExpression altogether? this can be parsed just fine as an identifier. It seems a bit arbitrary to parse this specially when we don't support other related parts of JS, like e.g. the new operator.

@LeaVerou LeaVerou added the bug label Sep 18, 2017
@EricSmekens
Copy link
Owner

I agree, we should have a look into this.

@LeaVerou
Copy link
Collaborator

I agree, we should have a look into this.

I guess we need to start by asking @soney how he feels about us removing it, since he added it pretty early on.

@gms1
Copy link

gms1 commented Oct 19, 2017

pros for keeping ThisExpression:

  1. evaluating an identifier node in the AST can be done by returning "context[node.name]". which would not work if 'this' is threated as identifier ( except you want to force AST consumers to do silly things like e.g. "context['this'] = context",... )

  2. it would be nice to keep jsep's AST output similar to esprima's

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

No branches or pull requests

4 participants