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

[Expression] Add support of create object via {key: value} syntax #3694

Merged
merged 10 commits into from
Apr 14, 2020

Conversation

cosmicshuai
Copy link
Contributor

@cosmicshuai cosmicshuai commented Apr 7, 2020

fixes: #3493
fixes: #3696

Now in LG and Expression, user can create an object by {key, value, ...}
For example:
{user: "Wilson"}, {user: {age: 27}}
Also, the value can be some variable defined in the scope,
For example, if the scope is {userName: "Allen"}
{user: userName} will be evaluated as {user: "Allen"}

@xieofxie
Copy link
Contributor

xieofxie commented Apr 7, 2020

Could we use variable as key in this grammar?

@Danieladu
Copy link
Collaborator

Could we use variable as key in this grammar?

As the discussion result, object key should be a constant and variable is definitely not supported, since the object definition is a syntactic sugar for user to create an initialized object quickly.
If you have the requirement to build an object with dynamic key, please use SetProperty or AddProperty instead.

@Danieladu
Copy link
Collaborator

Maybe we can add a new builtin function called createObject that accepts the params.
maybe:
solution 1:

createObject() -> {}
createObject('a', 'b', 'c', 'd')  -> {"a":"b", "c":"d"}

solution 2:

createObject(['a', 'b'], ['c', 'd'])  -> {"a":"b", "c":"d"}

Any ideas? @vishwacsena

@vishwacsena
Copy link
Contributor

@Danieladu I'm not sure I fully understand the createObject proposal. Especially not sure if you are proposing that as a way to address the dynamic key issue or as a general prebuilt function. Can you clarify?

@xieofxie can you share your intended use case/ scenario for key as a variable? Is this something you could achieve via SetProperties? @Danieladu @cosmicshuai can we make sure the below works for dynamic key?

new SetProperties()
{
    Assignments = new List<PropertyAssignment>()
    {
        new PropertyAssignment()
        {
            Property = "user.foo",
            Value = "={}"
        },
        new PropertyAssignment()
        {
            Property = "user.foo[dialog.dynamicKey]",
            Value = "bar"
        }
    }
}

@xieofxie
Copy link
Contributor

xieofxie commented Apr 9, 2020

No, I don't have a use case now. I am just curious.

SetProperty and AddProperty will do the work

@Danieladu
Copy link
Collaborator

@vishwacsena the SetProperities definitely works well, because it has nothing to do with this object creation feature.

Why we want to create a createObject function, consider these two points:

  1. Object creation expression (e.g. {a: 'b'}), should has a constant string as the key, and from another perspective, setproperty and addProperty function is not very suitable for processing nested objects. So, createObject is the right entry point.
  2. Object creation expression (e.g. {a: 'b'}) should depends on the built in functions to make it self work. Currently, For example:
    {key1: 'v1', key2: 'v2'} -> Equals to SetProperty(SetProperty(json('{}'), 'key1', 'v1'), 'key2', 'v2')

It is ugly, with endless nesting.

So, we want replace it as:
{key1: 'v1', key2: 'v2'} -> Equals to
createObject('key1', 'v1', 'key2', 'v2')
Or createObject(['key1', 'v1'], ['key2', 'v2'])

It is more readable.

@vishwacsena
Copy link
Contributor

It is sufficient if we get the inline notation to work for stable release. we can consider making a function like createObject public if the inline notation + setProperty route is not resonating with customers.

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

Successfully merging this pull request may close these issues.

=addProperty({} , "a" ,dialog.a) could not be executed twice Support array and json syntax in expressions
5 participants