Skip to content

Replace constructor with init#633

Merged
twokul merged 2 commits intomasterfrom
twokul/init
Nov 19, 2018
Merged

Replace constructor with init#633
twokul merged 2 commits intomasterfrom
twokul/init

Conversation

@twokul
Copy link
Contributor

@twokul twokul commented Nov 17, 2018

As per RFC 337, the way
classes are initialized has changed. Namely, arguments are no longer
available in constructor but they are still available in init.

Because we used constructor to set properties from passed in arguments
before, we started seeing error on Travis:

Error: Property set failed: object in path "unwrappedApi" could not be found.

This change switches to using init where appropriate and is a
backwards compatible change.

EDIT:

An unfortunate side-effect of replacing constructor with init is
that class properties are no longer assigned properly. They used to be
assigned after init. So all class properties initializers that are not decorated
with @argument need to be moved in init.

thanks to @pzuraq for brain dump and explaining me differences between init and constructor.

As per [RFC 337](emberjs/rfcs#337), the way
classes are initialized has changed. Namely, arguments are no longer
available in `constructor` but they are still available in `init`.

Because we used `constructor` to set properties from passed in arguments
before, we started seeing error on Travis:

```
Error: Property set failed: object in path "unwrappedApi" could not be found.
```

This change switches to using `init` where appropriate and is a
backwards compatible change.
An unfortunate side-effect of replacing `constructor` with `init` is
that class properties are no longer assigned properly. They used to be
assigned after `init`. So all class properties initializers that are not decorated
with `@argument` need to be moved in `init`.
@twokul twokul merged commit bac4635 into master Nov 19, 2018
@twokul twokul deleted the twokul/init branch November 19, 2018 18:58
@josemarluedke
Copy link
Contributor

@twokul Is it possible to get a new beta release containing these changes?

@twokul
Copy link
Contributor Author

twokul commented Dec 12, 2018

@josemarluedke will do by the end of the week

@twokul
Copy link
Contributor Author

twokul commented Dec 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants