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

User-defined types not normalized properly when passed in as data #4632

Closed
7 tasks
julianyus opened this issue Oct 13, 2016 · 9 comments
Closed
7 tasks

User-defined types not normalized properly when passed in as data #4632

julianyus opened this issue Oct 13, 2016 · 9 comments

Comments

@julianyus
Copy link
Contributor

julianyus commented Oct 13, 2016

Prerequisites

  • [x ] I have searched for similar issues in both open and closed tickets and cannot find a duplicate
  • [x ] The issue still exists against the latest master branch of Select2
  • [x ] This is not a usage question (Those should be directed to the community)
  • [x ] I have attempted to find the simplest possible steos to reproduce the issue
  • I have included a failing test as a pull request (Optional)

Steps to reproduce the issue

Initialize select 2 like this:

         var datos2 = [
                new producto(1, 'aaaaaa'),
                new producto(2, 'aaaaaa2'),
                new producto(3, 'aaaaaa3')
         ];

        $("#desplegable").select2({
            data: datos2
        });

Expected behavior and actual behavior

When I follow those steps I see that the text of the options is: [object Object]

I was expecting it to be: aaaaaa, aaaaaa2 and aaaaaa3

According to what I have been able to investigate, the problem is here:

SelectAdapter.prototype._normalizeItem = function (item) {
    if (!$.isPlainObject(item)) { <--- returns false
      item = {
        id: item,
        text: item
      };
    }
...
if (item.id != null) {
      item.id = item.id.toString(); <--- [object Object]
    }

Environment

Browsers

  • [x ] Google Chrome
  • [x ] Mozilla Firefox
  • [x ] Internet Explorer

Operating System

  • [x ] Windows
  • Mac OS X
  • Linux
  • Mobile

Libraries

  • jQuery version: 2.2.4
  • Select2 version: 4.0.3

Isolating the problem

  • This bug happens on the examples page
  • [x ] The bug happens consistently across all tested browsers
  • This bug happens when using Select2 without other pluigns
  • I can reproduce this bug in a jsbin
@alexweissman
Copy link
Contributor

This appears to be a duplicate of #4438, which was closed due to lack of activity. Can you create a jsbin that replicates this behavior?

I guess I don't know how jQuery decides when an object is a "plain object". I found this documentation on Plain objects, but it doesn't mention how it treats user-defined types (like the producto in your example).

@alexweissman alexweissman changed the title Problem with constructors User-defined types not normalized properly in templateSelection Oct 3, 2017
@julianyus
Copy link
Contributor Author

Hi,

I created a sample that recreates the problem: https://jsbin.com/riyamuyixi/edit?html,js,output

@alexweissman alexweissman changed the title User-defined types not normalized properly in templateSelection User-defined types not normalized properly when passed in as data Oct 4, 2017
@alexweissman
Copy link
Contributor

Thanks. I've made this fork which shows that the bug only affects user-defined types: https://jsbin.com/cufawefalu/edit?html,js,console,output

@julianyus
Copy link
Contributor Author

julianyus commented Oct 6, 2017

Hi,

What do you think about changing the call to isPlainObject by:

if(!(item === Object(item)))

Source: https://stackoverflow.com/a/14706877/1038133

I have tested with this data and all cases have worked::

 var datos2 = [
                "Test",
                {
                    id: 4,
                    text: 'item'
                },
                new producto(1, 'aaaaaa'),
                new producto(2, 'aaaaaa2'),
                new producto(3, 'aaaaaa3')
         ];

@alexweissman
Copy link
Contributor

This could be a good solution. Keep in mind that Select2 also supports grouped data, so this should probably be tested as well.

Can you submit a pull request that makes the change, and also adds some additional tests for these cases?

@julianyus
Copy link
Contributor Author

Hi,

I tested manually with grouped data and it worked

var datos3= [
    { 
      "text": "Group 1", 
      "children" : [
        {
            "id": 1,
            "text": "Option 1.1"
        },
        new producto(1, 'aaaaaa')
      ]
    },
    { 
      "text": "Group 2", 
      "children" : [
        {
            "id": 3,
            "text": "Option 2.1"
        },
        {
            "id": 4,
            "text": "Option 2.2"
        }
      ]
    }
  ];

Where dou you want me to put the tests? Should I made a new module?
I think that testing the _normalizeItem function from SelectAdapter with a string, a plain object and a user-defined object is enough because, after all, grouped data is a plain object with a property called "children".

@alexweissman
Copy link
Contributor

Awesome! So, automated tests are written with qunit, and go in https://github.com/select2/select2/tree/master/tests.

It seems like the test for user-defined types should go in either https://github.com/select2/select2/blob/master/tests/data/array-tests.js or https://github.com/select2/select2/blob/master/tests/data/select-tests.js (possibly both).

julianyus added a commit to julianyus/select2 that referenced this issue Oct 25, 2017
@julianyus julianyus mentioned this issue Oct 25, 2017
2 tasks
@alexweissman
Copy link
Contributor

#5093 fixes this, and has been merged in to the 4.0.6-rc.0 pre-release.

@vzhilov
Copy link

vzhilov commented May 9, 2020

In the last version 4.0.13 I had to comment that line out:

SelectAdapter.prototype._normalizeItem = function (item) {

    if (!$.isPlainObject(item)) { <--- returns false
      item = {
        id: item,
        text: item
      };
    }
...
if (item.id != null) {
//      item.id = item.id.toString(); <--- [object Object]
    }`

as my text object contans several fields like text : {name="foo", lastname="bar"} etc.

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

No branches or pull requests

3 participants