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

data-* attribute regression/bug since 0.15 #1976

Closed
5 tasks done
bago opened this issue Mar 1, 2023 · 6 comments
Closed
5 tasks done

data-* attribute regression/bug since 0.15 #1976

bago opened this issue Mar 1, 2023 · 6 comments

Comments

@bago
Copy link

bago commented Mar 1, 2023

  • Search for if my issue has already been submitted
  • Make sure I'm reporting something precise that needs to be fixed
  • Give my issue a descriptive and concise title
  • Create a minimal working example on JsFiddle or Codepen
    (or gave a link to a demo on the Selectize docs)
  • Indicate precise steps to reproduce in numbers and the result,
    like below

In 0.14 this code
<option data-data='{"url":"https://www.google.com"}'>value</option>
generated an "item.url" property.
In 0.15 this generates an "item.data.url" property instead (and also a buggy item.'{"url":"https://www.google.com"}' property )

You can see the bug in this jsbin (change the html code to include 0.14.0 and it will work fine):
https://jsbin.com/legilux/edit?html,js,console,output

Steps to reproduce:

  1. simply use a simple data-data attribute with a JSON and look at the item being passed to the rendering functions.
  2. e.g: value

Expected result:

item.url = "https://www.google.com"

Actual result:

item.data.url = "https://www.google.com"

The issue has been introduced in 111e6f3 breaking the data-data JSON handling at all. Then it has been refactored in c9ed4e8 but supporting data-* moved the data-data object from the root to the data subproperty.

I don't know if this is expected (i doubt the item.'{"url":"https://www.google.com"}' to be expected, anyway), but I guess if this is expected it should be documented in the changelog (or maybe I didn't find it).

Here is the code for readData before, in the middle and after the changes and the results of the call to that function:

var attr_data = 'data-data';

var isJSON = function (data) {
  try {
    JSON.parse(str);
  } catch (e) {
    return false;
  }
  return true;
};

var readDataOld = function($el) {
	var data = attr_data && $el.attr(attr_data);
	if (typeof data === 'string' && data.length) {
		return JSON.parse(data);
	}
	return null;
};

var readDataMid = function($el) {
	var data = attr_data && $el.attr(attr_data);
    var obj = {};

	if (typeof data === 'string' && data.length && isJSON(data)) {
		return JSON.parse(data);
    }

    obj[data] = data;

	return obj || null;
};

var readDataNew = function($el) {
    var data = attr_data && $el.attr(attr_data);
    var allData = $el.data();
    var obj = {};

	if (typeof data === 'string' && data.length) {
        if (isJSON(data)) {
          Object.assign(obj, JSON.parse(data))
        } else {
          obj[data] = data;
        }
    }

    Object.assign(obj, allData);

	return obj || null;
};



var el = $('<div data-data=\'{"url":"https://www.google.com"}\'></div>');
readDataOld(el); // {url: 'https://www.google.com'}
readDataMid(el); // {{"url":"https://www.google.com"}: '{"url":"https://www.google.com"}'}
readDataNew(el); // {{"url":"https://www.google.com"}: '{"url":"https://www.google.com"}', data: {url: 'https://www.google.com'}}
@mflorea
Copy link

mflorea commented Mar 10, 2023

This is an important regression (the documented dataAttr configuration doesn't work any more) that was introduced 4 months ago and then fixed one month later but not released. Are there any plans to release 0.15.3 anytime soon? We cannot upgrade to 0.15.2 with this regression. Thanks.

@bago
Copy link
Author

bago commented Mar 10, 2023

Update: I just noticed that my readDataNew doesn't mimic the latest code as another commit fixed the isJSON method:
8d805f6

This commit is about documentation so I didn't notice it, but it actually fixes the isJSON method:
https://github.com/selectize/selectize.js/blame/8d805f6158088ff68f69190cc21998f5452e0a64/src/utils.js#L383

Using the fixed isJSON the result for my test code seems to be ok:

{ url: "https://www.google.com/", data: { url: "https://www.google.com/" } }

@mflorea
Copy link

mflorea commented Mar 10, 2023

@bago yes, the problem in the isJSON function that uses the undefined str variable instead of the data input parameter, and it was actually introduced 5 months ago. It should be fixed on master, but we need a release.

@MartialSeron
Copy link

Hi. I just upgraded from the old 0.12 to 0.15.2.
I'm facing this issue. Any news on an upcoming release ?

@github-actions
Copy link
Contributor

github-actions bot commented Aug 6, 2023

This issue is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 5 days

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Aug 22, 2023
@bago
Copy link
Author

bago commented Aug 29, 2023

This bot automations are really bad for the community.
This bug should be closed once a release will be made with the code that fixed it.

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