-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Support nested JSON objects #53
Comments
Even if I was arguing with @devinus on the subject I am actually +1 on this :) |
I'm +1 on this as well. This would be especially nice for people using non-relational databases like couch. |
Or ElasticSearch in my case. |
Another idea: name: DS.attr({
first: DS.attr('string'),
last: DS.attr('string')
}) |
Hmm, the problem with this future is potential overhead on write. Read On 19 janv. 2012, at 20:02, Devin Torres
|
Is this for read-only cases or do you expect to write them back to the server as well? How do you detect when the top-level object is dirtied? You should be able to create multiple embedded models. Given the JSON data: {
name: "Steve",
address: {
street: "1 Infinite Loop",
country: {
short_name: "USA",
long_name: "United States"
}
}
} You should be able to model this with the following models: Country = DS.Model.extend({
shortName: DS.attr('string', { key: 'short_name' }),
longName: DS.attr('string', { key: 'long_name' })
});
Address = DS.Model.extend({
street: DS.attr('string'),
country: DS.hasOne(Country, { embedded: true })
});
Person = DS.Model.extend({
name: DS.attr('string'),
address: DS.hasOne(Address, { embedded: true })
}); Are you arguing that this is too much ceremony? It seems like you actually do want each of these conceptually to be separate models, so you can track dirty states separately. |
I'm arguing that it can end up being too much ceremony, yes. For example, sometimes I may get back nested JSON that doesn't map well to hasOne attributes. For example: {
name: {
first: 'Devin',
middle: 'Alexander',
last: 'Torres'
}
} Am I supposed to create a Name model for something like this? These things also aren't coming back with unique id's by the way, how does hasOne handle that? |
Since you are dealing with crazy, non-conventional JSON anyhow, couldn't you normalize the data in your adapter before you send it to the store? |
This kind of nested JSON is all over the place: CouchDB, ElasticSearch, Neo4j... I'd argue that there's nothing crazy or unconventional about it. I'd also argue normalizing data in an adapter isn't what an adapter should be for either. I've had thoughts of maybe introducing some type of Transformer, but really--shouldn't the model model your data and not the other way around? Your thoughts? |
+1 ... agree with @devinus . sometimes nested json properties are actually part of the "top-level object"... so then if any property inside of them changes, it would "dirty" the whole thing. and yes, for our app some of these nested-json sections are indeed being serialized and sent back to server currently, can't use hasOne because nested models dont have "primary keys". could fake it in adapter, as mentioned... but that's extra work to make two unrelated concepts fit together (these aren't separate "database objects", which is really what hasOne/hasMany is doing for us.... so I'd be working around the framework) will find another workaround for the timebeing... |
Are you willing to write out the entire object when it changes, as opposed to being able to modify the object directly? Something like: Person = DS.Model.extend({
attrs: DS.attr('object')
});
var person = Person.find(1);
var attrs = person.modify('attrs', function(attrs) {
attrs.age++;
});
// this would be illegal
person.get('attrs').age++; The reason for this is that if you modify a conceptually whole attribute by mutating it directly, we don't know that the attribute has changed. We don't want to observe changes to the hash directly, because that may mutate the object in ways that harm its ability to be serialized back (for example, observers add on ES5 accessors to warn users who try to access them directly that they should be going through set/get; even without that, the object needs to be annotated with some extra observability information, which changes it from being a raw, easily serializable object to an extended object). With that said, would you be comfortable with a Suboptimal all around. |
Another idea is to return a proxy when you request the object (with This would have the benefit of being mostly transparent, but @tomdale is still skeptical. I think it can work ;) |
@wycats You're assuming we're only working with JSON that's nested one level deep. |
I understand what you're saying... and agree that it is suboptimal. I think hasOne() actually works pretty closely enough... it would give me the control to build a Model for my nested JSON, which allows me to determine what sub-properties are being observed and how... if I want to do that. but currently that pattern:
feels like there would be a way to change the association code a bit (hasOne, hasMany)... so then you're not inventing a DS.attr('object') where you dont exactly know what the developer wants you to do with it. we can make an "object" transform now... and do the work there on full set/get... but it feels like the tools are already here for the developer to be explicit with the model schema using DS.Model and build up a nested schema as complicated as they want |
looks like a duplicate: #100 |
@devinus nope. note that I said "or another proxy if it was nested data". @NickFranceschina I also thought about reusing the hasOne code, but note that hasOne doesn't mark the parent object as dirty. Additionally, it wouldn't work for nested objects without even more shenanigans (you would want changes, however deep, to mark the parent as dirty). I think the proxy solution is the best solution we have so far. |
@wycats Ah, my bad. |
I think that there is still a case for treating primitive arrays attributes seperate from object attributes. We can use syntax like DS.attr('date[]') and easily use transforms and manage dirty state. #96 |
+1 I'm about to start a project using couchdb. It makes a lot of sense to have |
+1 |
We haven't forgotten about this issue :D |
bump to restart discussion... |
What's the current status of this issue, looks like it's effectively dead? If the issue won't be fixed, what is the recommended workaround? |
https://github.com/lytics/ember-data.model-fragments was the way to go for us, @lolmaus |
@elliterate, model-fragments claims to be incompatible with the current version of Ember Data. I also find it risky to depend on a third party extension that hacks Ember internals: even after becoming compatible, it might break with next update of Ember Data. I managed to come up with a solution using native Ember features: http://emberjs.jsbin.com/vapama/1/edit?html,js,output Highlights:
The latter will require a lot of utility code, so don't hesitate to suggest a more concise approach if you have something in mind. |
Here's a slightly more compact version: http://emberjs.jsbin.com/vapama/3/edit?html,js,output Instead of having the checkbox ask the controller to stain the record, the checkbox stains the record itself. |
An even shorter version, now reusing the input helper rather than subclassing it: http://emberjs.jsbin.com/vapama/4/edit?html,js,output Can you guys please review this and tell your opinion on whether it's an appropriate solution? UPD: http://emberjs.jsbin.com/vapama/5/edit?html,js,output
UPD2: http://emberjs.jsbin.com/vapama/8/edit?html,js,output
|
Okay, this works in a JSBin, but in real-life scenario this approach requires so much utility code that it becomes simpler, more error-proof and definitly more Ember-way to use nested models. |
Yay, Model Fragments have been updated to support Ember Data beta 14! :D |
I'm also trying to do a |
@JKGisMe I have the same need in my app for which model fragments works well. I understand your reluctance to add more dependencies, but model fragments was born out of your exact need. Ember Data may one day support nested id-less records like this, but that day is far in the future. If you are interested, this is the rough pattern I'm using:
export default DS.Model.extend({
config: DS.hasManyFragments('config_field')
});
export default DS.ModelFragment.extend({
name: DS.attr('string'),
value: DS.attr('string')
});
{{each config as |config|}}
<div class="form-control">
<label>{{config.name}}</label>
{{input value=config.value}}
</div>
{{/each}} This will serialize
export default DS.RESTSerializer.extend({
serializeAttribute: function(snapshot, json, key) {
if (key === 'config') {
json.config = snapshot.attr('config').reduce(function(config, fieldSnapshot) {
config[fieldSnapshot.attr('name')] = fieldSnapshot.attr('value');
return config;
}, {});
} else {
this._super.apply(this, arguments);
}
}
}); |
👍 for model fragments |
Hi people. I'm following this approach. In my case I want to store a info object for a channel entity I'm working on: var Channel = DS.Model.extend({
name: DS.attr('string'),
info: DS.attr({ defaultValue: () => Info.create() })
});
var Info = Ember.Object.extend({
version: DS.attr('string'),
description: DS.attr('string')
}); What is bad about this approach? Thank you. |
Well, I have no idea what happens when you put a DS.attr inside something that is not a |
Thank you @slindberg I didn't see this earlier!. That looks very clean and probably would have worked great. The specs for our project changed a little and I ended up just making a whole new model & accompanying component in Rails & Ember which I then convert to customized JSON in the end from Rails. |
Is ember-data-model-fragments now incompatible with ember-data |
It will be compatible soon |
Hello guys, I followed this discussion because I have the same use case (nested documents). Having said that, I vote against having model-fragments in ember-data. |
@boehlke You assume that any data can be flattened, but that's absolutely not the case. |
@lolmaus Sorry, I cannot think of an example JSON object which cannot be flattened. Can you provide one? Probably that's easier than for me to prove that every JSON can be flattened. Thanks! |
This thread is turning into masturbation. Embedded objects are parts of JSON documents. If ember won't support it, fuck it. This framework is too "idiomatic" to be usable. |
@boehlke Imagine a hash that has arrays which contain hashes which contain arrays of hashes. You can of course undestructively flatten it by producing really elaborate key names. But working with such a flat structure from Ember will be a nightmare. You won't be able iterate over specific sub-arrays, etc. That's just not a working solution. |
The reality is, depending on the scenario people want both flattened and nested structures are ideal. Compromises exists, but as ED's goal was consistency, that explains its current state (not its final form).
Ember works fine with deeply nested data-structures out of the box. Ember-data also works with deeply nested documents. I believe the pain people are noticing. Is they want there nested blobs to behave like ember-data models, in both a read/write/serializer/deserialize fashion. Ember-data today, is largely the following units (on top of ember), and models have nice patterns, such as:
Expanding these patterns to the nested id'less entities, is something we are interested in exploring. the model-fragments project is showing promise, and is a very intriguing interpretation of the problem. Unfortunately, it relies on private API and monkey patching to accomplish its goal. One of our goals moving forward is, to ensure a stable public API for model-fragments. This will prevent the upgrade turmoil currently introduced. Now as for model-fragments becoming part of ember-data itself, it's too soon to know for sure. It is great to push this logic into user-space, as it will come with some caveats (and the problem space needs some more fleshing out) The important part today is for both model-fragments and ember-data to work better in tandem. This will allow the flexibility for authors to experiment, but also the stability to ship real projects with it. |
@wildchild I would advise you to read the community guidelines. I do not mean to belittle your frustration but this kind of behavior is not acceptable in the ember community. http://emberjs.com/guidelines/ |
I just ran into this last night:
Because I am forced to do this as a workaround in if (isNone(get(model, 'templateSettings.context.hero'))) {
set(model, 'templateSettings.context.hero', {});
} |
@devinus a better place would be to add that to your serialization code. Also, those questions are better asked on SO. |
@stefanpenner What question? (Also, this is before serialization even happens. I was just providing a counter-example to your claim that "Ember works fine with deeply nested data-structures out of the box." In my experience, it does not.) |
@lolmaus I'm having trouble figuring out how to nicely flatten a JSON. |
@aivanov93 ember-data-model-fragments seem to fit nicely into your use case. But I would attempt a completely different solution. I would split the model into two parts, e. g. {
id: '1'
type: 'post',
relationships: {
translations: [
{id: '1', type: 'post-translation'},
{id: '2', type: 'post-translation'}
]
}
}
{
id: '1'
type: 'post-translation',
relationships: {
post: {id: '1', type: 'post'}
},
attributes: {
lang: 'en',
title: 'Hello world!',
body: "Wonderful weather we have today, don't we?"
}
}
{
id: '2'
type: 'post-translation',
relationships: {
post: {id: '1', type: 'post'}
},
attributes: {
lang: 'ru',
title: 'Здравствуй, мир!',
body: 'Чудесная погодка сегодня, не так ли?'
}
} This approach will also let you collect language-specific comments to posts. |
# This is the 1st commit message: WIP Tracerbench Travis setup # This is the commit message #2: dont checkin results # This is the commit message #3: configure azure job, make first thing for testing # This is the commit message #4: config updates # This is the commit message #5: Able to get results from Tracerbench # This is the commit message #6: redirect for tracerbench # This is the commit message #7: Update URLs to include ?tracerbench=true # This is the commit message #8: wip # This is the commit message #9: moves .server-replay.json to root # This is the commit message #10: wip # This is the commit message #11: cleanup # This is the commit message #12: cleanup # This is the commit message #13: cleanup # This is the commit message #14: cleanup # This is the commit message #15: change command # This is the commit message #16: Make process terminate with exit code 0 # This is the commit message #17: Ignore tracerbench-results files # This is the commit message #18: Delete generated JSON file # This is the commit message #19: rename directory # This is the commit message #20: Bump ember-decorators-polyfill from 1.0.6 to 1.1.0 Bumps [ember-decorators-polyfill](https://github.com/pzuraq/ember-decorators-polyfill) from 1.0.6 to 1.1.0. - [Release notes](https://github.com/pzuraq/ember-decorators-polyfill/releases) - [Commits](ember-polyfills/ember-decorators-polyfill@1.0.6...1.1.0) Signed-off-by: dependabot-preview[bot] <[email protected]> # This is the commit message #21: Bump @ember/optional-features from 1.0.0 to 1.1.0 Bumps [@ember/optional-features](https://github.com/emberjs/ember-optional-features) from 1.0.0 to 1.1.0. - [Release notes](https://github.com/emberjs/ember-optional-features/releases) - [Changelog](https://github.com/emberjs/ember-optional-features/blob/master/CHANGELOG.md) - [Commits](emberjs/ember-optional-features@v1.0.0...v1.1.0) Signed-off-by: dependabot-preview[bot] <[email protected]> # This is the commit message #22: [CHORE] fix Typo (emberjs#6635) Happen to run into this typo. Been using Ember for years, love your work! Good luck! Co-authored-by: igorT <[email protected]> # This is the commit message #23: Update Changelog for v3.13.0 # This is the commit message #24: update changelog for v3.13.1 # This is the commit message #25: [CHORE] adds infra for testing calls to Ember warn|deprecate|assert (emberjs#6626) * [CHORE] adds infra for testing calls to Ember warn|deprecate|assert * fix production tests * address feedback * feedback on labels * remove id checks # This is the commit message #26: [CHORE] Refactor integration/multiple-stores-test.js in order to remove run loop usage (emberjs#6632) # This is the commit message #27: Bump ember-simple-tree from 0.7.0 to 0.7.1 Bumps [ember-simple-tree](https://github.com/btecu/ember-simple-tree) from 0.7.0 to 0.7.1. - [Release notes](https://github.com/btecu/ember-simple-tree/releases) - [Commits](btecu/ember-simple-tree@0.7.0...0.7.1) Signed-off-by: dependabot-preview[bot] <[email protected]> # This is the commit message #28: Bump ember-decorators-polyfill from 1.1.0 to 1.1.1 Bumps [ember-decorators-polyfill](https://github.com/pzuraq/ember-decorators-polyfill) from 1.1.0 to 1.1.1. - [Release notes](https://github.com/pzuraq/ember-decorators-polyfill/releases) - [Commits](ember-polyfills/ember-decorators-polyfill@1.1.0...1.1.1) Signed-off-by: dependabot-preview[bot] <[email protected]> # This is the commit message #29: cleanup serializer documentation (emberjs#6575) # This is the commit message #30: ISSUE-6012: test: added test for meta property and bug fix. (emberjs#6640) # This is the commit message #31: [FEAT links] ensures full links object support for relationships (emberjs#6606) * [FEAT links] ensures full links object support for relationships * fix lint * add feature flags # This is the commit message #32: [BUGFIX release] Enable `store.createRecord` in FastBoot (emberjs#6568) * Add FastBoot test for store.createRecord Fails at the moment. * Enable store.createRecord in FastBoot Changes the V4 UUID generation to leverage `FastBoot.require` when possible. This currently requires that the host application add the following to their `package.json`: ``` "fastbootDependencies": [ "crypto" ] ``` * fix types # This is the commit message #33: [CHORE] rename and restructure packages to clarify unpublished/private status move unpublished packages into directories marked private move published but private into better naming convention move unpublished into better naming convention change -build-infra imports to private-build-infra larger re-org # This is the commit message #34: Bump eslint-config-prettier from 6.4.0 to 6.5.0 Bumps [eslint-config-prettier](https://github.com/prettier/eslint-config-prettier) from 6.4.0 to 6.5.0. - [Release notes](https://github.com/prettier/eslint-config-prettier/releases) - [Changelog](https://github.com/prettier/eslint-config-prettier/blob/master/CHANGELOG.md) - [Commits](prettier/eslint-config-prettier@v6.4.0...v6.5.0) Signed-off-by: dependabot-preview[bot] <[email protected]> # This is the commit message #35: Bump eslint from 6.5.1 to 6.6.0 Bumps [eslint](https://github.com/eslint/eslint) from 6.5.1 to 6.6.0. - [Release notes](https://github.com/eslint/eslint/releases) - [Changelog](https://github.com/eslint/eslint/blob/master/CHANGELOG.md) - [Commits](eslint/eslint@v6.5.1...v6.6.0) Signed-off-by: dependabot-preview[bot] <[email protected]> # This is the commit message #36: [CHORE] refactor: Remove runloop usage in destroy module of integration/store… (emberjs#6646) * refactor: Remove runloop usage in destroy module of integration/store-test * fix linter errors # This is the commit message #37: Bump @types/ember__debug from 3.0.5 to 3.0.6 (emberjs#6647) Bumps [@types/ember__debug](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/ember__debug) from 3.0.5 to 3.0.6. - [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases) - [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/ember__debug) Signed-off-by: dependabot-preview[bot] <[email protected]> # This is the commit message #38: Bump eslint-plugin-mocha from 6.2.0 to 6.2.1 Bumps [eslint-plugin-mocha](https://github.com/lo1tuma/eslint-plugin-mocha) from 6.2.0 to 6.2.1. - [Release notes](https://github.com/lo1tuma/eslint-plugin-mocha/releases) - [Changelog](https://github.com/lo1tuma/eslint-plugin-mocha/blob/master/CHANGELOG.md) - [Commits](lo1tuma/eslint-plugin-mocha@6.2.0...6.2.1) Signed-off-by: dependabot-preview[bot] <[email protected]> # This is the commit message #39: Bump qunit-dom from 0.9.0 to 0.9.1 Bumps [qunit-dom](https://github.com/simplabs/qunit-dom) from 0.9.0 to 0.9.1. - [Release notes](https://github.com/simplabs/qunit-dom/releases) - [Changelog](https://github.com/simplabs/qunit-dom/blob/master/CHANGELOG.md) - [Commits](mainmatter/qunit-dom@v0.9.0...v0.9.1) Signed-off-by: dependabot-preview[bot] <[email protected]> # This is the commit message #40: Bump @typescript-eslint/parser from 2.5.0 to 2.6.0 Bumps [@typescript-eslint/parser](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/parser) from 2.5.0 to 2.6.0. - [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases) - [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/parser/CHANGELOG.md) - [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v2.6.0/packages/parser) Signed-off-by: dependabot-preview[bot] <[email protected]> # This is the commit message #41: Bump @typescript-eslint/eslint-plugin from 2.5.0 to 2.6.0 Bumps [@typescript-eslint/eslint-plugin](https://github.com/typescript-eslint/typescript-eslint/tree/HEAD/packages/eslint-plugin) from 2.5.0 to 2.6.0. - [Release notes](https://github.com/typescript-eslint/typescript-eslint/releases) - [Changelog](https://github.com/typescript-eslint/typescript-eslint/blob/master/packages/eslint-plugin/CHANGELOG.md) - [Commits](https://github.com/typescript-eslint/typescript-eslint/commits/v2.6.0/packages/eslint-plugin) Signed-off-by: dependabot-preview[bot] <[email protected]> # This is the commit message #42: fix tracerbench command for renamed test app folder # This is the commit message #43: Setup har-remix # This is the commit message #44: replace server-replay with har-remix # This is the commit message #45: Use concurrently from node_modules instead of global installation # This is the commit message #46: wip # This is the commit message #47: Update bash script to use ember-data workspace for experiment # This is the commit message #48: relationship performance bash script updates # This is the commit message #49: removes line breaks # This is the commit message #50: removes .travis.yml from test app # This is the commit message #51: cleanup linting # This is the commit message #52: change eslint config # This is the commit message #53: cleanup # This is the commit message #54: remove comments # This is the commit message #55: updates azure-pipelines.yml # This is the commit message #56: adds tracerbench to github action workflow # This is the commit message #57: Update CI to install PM2 globally # This is the commit message #58: [CHORE] allow asserting all tests for deprecations (emberjs#6627) * [CHORE] configure the ability to filter deprecations from assertNoDeprecations * turn off helpers until tests are fixed # This is the commit message #59: [DOC] Close code block in the description # This is the commit message #60: Bump ember-qunit from 4.5.1 to 4.6.0 Bumps [ember-qunit](https://github.com/emberjs/ember-qunit) from 4.5.1 to 4.6.0. - [Release notes](https://github.com/emberjs/ember-qunit/releases) - [Changelog](https://github.com/emberjs/ember-qunit/blob/master/CHANGELOG.md) - [Commits](emberjs/ember-qunit@v4.5.1...v4.6.0) Signed-off-by: dependabot-preview[bot] <[email protected]> # This is the commit message #61: Bump ember-fetch from 6.7.1 to 6.7.2 Bumps [ember-fetch](https://github.com/ember-cli/ember-fetch) from 6.7.1 to 6.7.2. - [Release notes](https://github.com/ember-cli/ember-fetch/releases) - [Changelog](https://github.com/ember-cli/ember-fetch/blob/v6.7.2/CHANGELOG.md) - [Commits](ember-cli/ember-fetch@v6.7.1...v6.7.2) Signed-off-by: dependabot-preview[bot] <[email protected]> # This is the commit message #62: Bump ember-load-initializers from 2.1.0 to 2.1.1 Bumps [ember-load-initializers](https://github.com/ember-cli/ember-load-initializers) from 2.1.0 to 2.1.1. - [Release notes](https://github.com/ember-cli/ember-load-initializers/releases) - [Changelog](https://github.com/ember-cli/ember-load-initializers/blob/master/CHANGELOG.md) - [Commits](ember-cli/ember-load-initializers@v2.1.0...v2.1.1) Signed-off-by: dependabot-preview[bot] <[email protected]> # This is the commit message #63: Bump ember-source from 3.13.3 to 3.14.1 Bumps [ember-source](https://github.com/emberjs/ember.js) from 3.13.3 to 3.14.1. - [Release notes](https://github.com/emberjs/ember.js/releases) - [Changelog](https://github.com/emberjs/ember.js/blob/master/CHANGELOG.md) - [Commits](emberjs/ember.js@v3.13.3...v3.14.1) Signed-off-by: dependabot-preview[bot] <[email protected]> # This is the commit message #64: Bump eslint-plugin-ember from 7.2.0 to 7.3.0 Bumps [eslint-plugin-ember](https://github.com/ember-cli/eslint-plugin-ember) from 7.2.0 to 7.3.0. - [Release notes](https://github.com/ember-cli/eslint-plugin-ember/releases) - [Changelog](https://github.com/ember-cli/eslint-plugin-ember/blob/master/CHANGELOG.md) - [Commits](ember-cli/eslint-plugin-ember@v7.2.0...v7.3.0) Signed-off-by: dependabot-preview[bot] <[email protected]>
E.g.:
Thoughts:
name: DS.attr('object')
firstName: DS.attr('string', { key: 'name.first' })
Justification:
Nested JSON objects are too prevalent to require transforming them every time they're used. Models should model the data, not some transformed representation you morph in adapter, and hopefully not through hundreds of transformers.
The text was updated successfully, but these errors were encountered: