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

Initialisation of Record Map #40

Open
irisSchaffer opened this issue Aug 16, 2017 · 3 comments
Open

Initialisation of Record Map #40

irisSchaffer opened this issue Aug 16, 2017 · 3 comments

Comments

@irisSchaffer
Copy link
Contributor

Hey there!

Great library, I just have one little problem/question about this line of code: see https://github.com/glenjamin/transit-immutable-js/blob/master/index.js#L215
Is there a reason for initialising the Records there? Or rather: is there a reason for initialising them with an empty Object ({})?

In my case, I have Records with a constructor and because of the initialisation of the Record with {}, I can't use default parameters ({ meta : {} }) and need to check if my nested object structures exist everywhere (data.meta && data.meta.title || ...):

export default class StartPage extends StartPageRecord {
  constructor(data = { meta : {} }, name) { // this here won't work
    super({
      ...data,
      selected : new Set(data.selected),
      image    : new Image(data.image),
      meta     : new Meta({
        title       : data.meta.title || data.title, // these throw errors
        description : data.meta.description || data.subtitle // because data.meta is undefined
      }),
      socialLinks : new SocialLinks(data.socialLinks)
    }, name)
  }
}
@irisSchaffer
Copy link
Contributor Author

If anybody else has this problem, I have created a fork in which the records get initialised with () instead: https://github.com/irisSchaffer/transit-immutable-js

@glenjamin
Copy link
Owner

I expect I would have done this because a record wouldn't instantiate without the values, but that doesn't seem to be the case anymore (if it even ever was).

i'll happily take a PR which changes this.

@irisSchaffer
Copy link
Contributor Author

See #42 :)

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

No branches or pull requests

2 participants