Skip to content

fix(compiler-core): No access this through function declaration in <script setup> #6827

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.

Sign up for GitHub

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
+46 −2
Open

fix(compiler-core): No access this through function declaration in <script setup> #6827

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d096f8c
fix(compiler-core): No access `this` through function declaration in …
godxiaoji Oct 6, 2022
4d90041
test: add test case
godxiaoji Oct 6, 2022
4314c78
fix(compiler-core): No access `this` through function declaration in …
godxiaoji Oct 6, 2022
d8488e9
test: update test case
godxiaoji Oct 6, 2022
0abab9c
Empty-Commit
godxiaoji Oct 6, 2022
ff87325
test: add test case
godxiaoji Oct 6, 2022
7b5fcbe
Merge branch 'main' into fix-6822
edison1105 Oct 22, 2024
6ebd836
[autofix.ci] apply automated fixes
autofix-ci[bot] Oct 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions packages/compiler-core/__tests__/transforms/transformExpressions.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -492,6 +492,7 @@ describe('compiler: expression transform', () => {
props: BindingTypes.PROPS,
setup: BindingTypes.SETUP_MAYBE_REF,
setupConst: BindingTypes.SETUP_CONST,
setupLet: BindingTypes.SETUP_LET,
data: BindingTypes.DATA,
options: BindingTypes.OPTIONS
}
Expand Down Expand Up @@ -531,5 +532,15 @@ describe('compiler: expression transform', () => {
expect(code).toMatch(`_ctx.options`)
expect(code).toMatchSnapshot()
})

// #6822
test('no access this through function declaration', () => {
const { code } = compileWithBindingMetadata(
`<div @click="setupConst()" @keyup="setupLet()" @keydown="setup()"></div>`
)
expect(code).toMatch(`$setup.setupConst.bind()`)
expect(code).toMatch(`$setup.setupLet.bind()`)
expect(code).not.toMatch(`$setup.setup.bind()`)
})
})
})
11 changes: 10 additions & 1 deletion packages/compiler-core/src/transforms/transformExpression.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,16 @@ export function processExpression(
} else {
if (type && type.startsWith('setup')) {
// setup bindings in non-inline mode
return `$setup.${raw}`
if (
parent &&
parent.type === 'CallExpression' &&
(type === BindingTypes.SETUP_LET || type === BindingTypes.SETUP_CONST)
Copy link
Member

@sxzz sxzz Oct 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Why check BindingTypes here? Consider this example, the problem is still existing.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. I had to make sure it's a function identifier with parents.type. Then use .bind() to change the scope to undefined.
  2. Considered with the smallest dimension of the issue, including the following situations when type is BindingTypes.SETUP_CONST or BindingTypes.SETUP_LET:
// BindingTypes.SETUP_LET
let foo = () => {}
let foo = function() {}

// BindingTypes.SETUP_CONST
const foo = () => {}
const foo = function {}
function foo() {}

Copy link
Member

Choose a reason for hiding this comment

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

There are many kinds of BindingTypes. For generality, I don't think it's necessary to check BindingTypes here.

Or if you have a better solution to fix the example I provided, it's also all right.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated! The previous judgment was poor, and the following conditions are also met.

const foo = ref(function(a, b) {
  console.log(this)
})

Copy link
Member

Choose a reason for hiding this comment

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

Great, but CI is failed. You can push an empty commit git commit --allow-empty -m "Empty-Commit" to re-run CI.

) {
// #6822 No access `this` through function declaration in <script setup>
return `$setup.${raw}.bind()`
} else {
return `$setup.${raw}`
}
} else if (type === BindingTypes.PROPS_ALIASED) {
return `$props['${bindingMetadata.__propsAliases![raw]}']`
} else if (type) {
Expand Down