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

fix: call getter accumulator with proper this context #31

Merged
merged 2 commits into from
Apr 15, 2023

Conversation

Julien-R44
Copy link
Member

Proposed changes

Without this fix, then the following code did not work:

class Foo extends Macroable {
  public value = "foo";
}

Foo.getter("bar", function (this: Foo) {
  console.log(this); // print undefined
  return this.value;
});

const foo = new Foo();
console.log(foo.bar);

In other words: the this context was not passed to the getters callback. I added a test in the PR that showed this behavior.
This also caused an undesired breaking change

@Julien-R44 Julien-R44 added the Type: Bug The issue has indentified a bug label Apr 14, 2023
@Julien-R44 Julien-R44 requested a review from thetutlage April 14, 2023 21:48
@Julien-R44 Julien-R44 self-assigned this Apr 14, 2023
@thetutlage thetutlage merged commit b4707ce into next Apr 15, 2023
@thetutlage
Copy link
Member

Thanks 👍

@Julien-R44 Julien-R44 deleted the fix/macroable-getter-this branch April 15, 2023 10:11
thetutlage added a commit that referenced this pull request Oct 14, 2023
* feat: rewrite from scratch

* docs: update links to badges

* ci: run tests for lts and latest version of Node

* test: fix breaking test

* chore(release): 1.0.0-0

* chore: update dependencies

* chore: make tsconfig strict

* chore: track coverage

* docs: improvements to examples

* chore(release): 1.0.0-1

* chore: update dependencies

* ci: use reusable workflow

* chore: update dependencies

* refactor: export Macroable as default

Breaking change: Single values should always be exported as default export

* chore(release): 1.0.0-2

* chore: update dependencies

* docs(README): fix badge url for github workflow

* chore: update dependencies

* chore(release): 1.0.0-3

* chore: update dependencies

* docs(README): update codeblocks

* chore(release): 1.0.0-4

* fix: call getter accumulator with proper `this` context (#31)

* fix: call getter accumulator with proper this context

* style: remove unused import

* chore: update dependencies

* chore(release): 1.0.0-5

* chore: update dependencies

* feat: allow re-assigning getters

* chore(release): 1.0.0-6

* chore: update dependencies

* chore: upgrade japa to v3

* chore: use @adonisjs/tooling presets for tooling config

* ci: add linting and typechecking in ci

* test: add expect-type plugin

* chore: add engines to package.json file

* chore(release): 1.0.0-7

* chore: update dependencies

* docs: update README

* chore(release): 1.0.0-8

* chore: update dependencies

* chore: publish under latest tag

* chore: use tsup for bundling

---------

Co-authored-by: Julien Ripouteau <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug The issue has indentified a bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants