A test case to show how to use a symbol as an attribute name #1626
Conversation
| class TestSchema extends DefaultSchema { | ||
| includesModel(modelName) { | ||
| return /^com.example.bookstore\./i.test(modelName); | ||
| } | ||
| setAttribute(modelName, attr, value, schemaInterface) { | ||
| if (attr === 'name') { | ||
| schemaInterface.setAttr('title', value); | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
I don't think we're opting in to the native proxy here.
You need something like
useNativeProperties() { return true; }There was a problem hiding this comment.
This setting seems to be not necessary in this use case, but it also doesn't hurt too.
| const mySymbol = Symbol('mySymbol'); | ||
| book.set(mySymbol, book.id); | ||
|
|
||
| assert.equal(book.get(mySymbol), 'urn:li:book:1', 'Reading a symbol attribute should work'); |
There was a problem hiding this comment.
We should be testing the native proxy and not calling get / set but rather
book[mySymbol] = book.id
assert.equal(book[mySymbol], 'urn:li:book:1')There was a problem hiding this comment.
Good point! If we opt to use this format, the test case can pass successfully without any other change in other files.
Do you think we should simply add something in README to the effect like the below:
When using symbols as attribute names, please be sure to use the format of model[mySymbol]. set() and get() are known to have issues.
Another option is me/somebody else to fix the proxy in ember, so that both the .[] and set/get can be used interchangeably.
There was a problem hiding this comment.
@larry-x-yu I don't think we need docs for symbols specifically.
The thing we can do to make things better is to drop CoreObject as a superclass in model, but it'll be a breaking change.
9ca404f to
bbc9cd6
Compare
708afca to
6b878ac
Compare
[Fix #1569]
Summary of change
Testing
All test cases (yarn test) passed in local setup.