Sitelet https://github.com/acornjs/acorn/pull/1361
Skip to content

Remove expression property from Function{Declaration,Expression} - #1361

Open
fisker wants to merge 2 commits into
acornjs:masterfrom
fisker:function-expression
Open

fisker wants to merge 2 commits into
acornjs:masterfrom
fisker:function-expression

Conversation

@fisker

@fisker fisker commented May 19, 2025 •

Copy link
Copy Markdown
Contributor

expression property on FunctionDeclration,FunctionExpression, and ArrowFunctionExpreesion is deprecated in ESTree https://github.com/estree/estree/blob/master/deprecated.md#functions long time ago

@fisker
fisker marked this pull request as draft May 19, 2025 06:46
Comment thread acorn/src/expression.js

if (isExpression) {
node.body = this.parseMaybeAssign(forInit)
node.expression = true

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm sorry if I'm wrong, but I think the expression property is required for ArrowFunctionExpression.

https://github.com/estree/estree/blob/master/es2015.md#arrowfunctionexpression

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! Realized that after I open this, so I put this into draft. And opened an issue about it in ESTree estree/estree#323

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right ArrowFunctionExpression.expression is required.

@fisker
fisker force-pushed the function-expression branch from 14a6c9d to 95d92d2 Compare May 20, 2025 03:11
@fisker fisker changed the title Remove Function.expression Remove expression property from Function{Declaration,Expression} May 20, 2025
@fisker
fisker marked this pull request as ready for review May 20, 2025 03:39
@marijnh

marijnh commented May 20, 2025

Copy link
Copy Markdown
Member

I feel it's likely that some users of the library are likely to be relying on this (either because their code is old, or because they found the property in the output and decided to use it). I know acorn-walk does, for one thing. As such, I do not think making this change in a non-major release would be a good idea.

@fisker

fisker commented May 20, 2025 •

Copy link
Copy Markdown
Contributor Author

To clarify, ArrowFunctionExpression.expression indicates the function body is an Expression which is not removed.

And Function{Declaration,Expression}.expression is always false.

I know acorn-walk does

Are you referring this line ?

c(node.body, st, node.expression ? "Expression" : "Statement")

It should still work the same as before.

Anyway, I understand your concern, it's acceptable if you want to do this in a major release.

demonsfiji

This comment was marked as spam.

0953599921

This comment was marked as spam.

@Anilu743

This comment was marked as spam.

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

Successfully merging this pull request may close these issues.

6 participants