Skip to content

Commit

Permalink
feat: Use WeakMap to manage list of registered elements
Browse files Browse the repository at this point in the history
Referring to data attributes is not safe, because the HTML
of the page can be mutated by other code. Using direct reference
to the elements in a weak map should let us safely test for
active widgets, and also allow garbage collection
  • Loading branch information
wheeyls committed Mar 18, 2024
1 parent ae62b4b commit dca5571
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 6 deletions.
32 changes: 29 additions & 3 deletions src/initWidgets.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,30 @@ import { extractOptions } from '@g2crowd/extract-options';
import camelize from './camelize';
import { strategies } from './strategies';

const widgetTracker = function () {
const elements = new WeakMap();

return {
get: function (widgetName, element) {
const widgets = elements.get(element);

return widgets && widgets.get(widgetName);
},

set: function (widgetName, element, value) {
if (!elements.has(element)) {
elements.set(element, new Map());
}

elements.get(element).set(widgetName, value);
},

has: function (widgetName, element) {
return this.get(widgetName, element) !== undefined;
}
};
};

function simpleCaste(value) {
try {
return JSON.parse(value);
Expand Down Expand Up @@ -38,14 +62,14 @@ const wrapPlugin = function wrapPlugin(name, pluginFn, element) {
};
};

const loadWidget = function (element, name, widgetQueue, registered) {
const existingPlugin = element.dataset[`vvidget_${camelize(name)}`];
const loadWidget = function (element, name, widgetQueue, registered, initiatedWidgets) {
const pluginFn = registered[name];

if (!pluginFn) {
return;
}

const existingPlugin = initiatedWidgets.get(name, element);
const wrapped = wrapPlugin(name, pluginFn, element);

if (!existingPlugin) {
Expand All @@ -54,10 +78,12 @@ const loadWidget = function (element, name, widgetQueue, registered) {
});
widgetQueue.flush();

initiatedWidgets.set(name, element, true);
element.dataset[`vvidget_${camelize(name)}`] = true;
}
};

const initiatedWidgets = widgetTracker();
export const widgetInitiator = function ({ attr, data, registered }, fn = loadWidget) {
const widgetQueue = queue();
registered = registered || {};
Expand All @@ -69,7 +95,7 @@ export const widgetInitiator = function ({ attr, data, registered }, fn = loadWi
names
.split(' ')
.filter((i) => i)
.forEach((name) => fn(element, name, widgetQueue, registered));
.forEach((name) => fn(element, name, widgetQueue, registered, initiatedWidgets));
});
};
};
9 changes: 6 additions & 3 deletions test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,10 @@ describe('with attr=ue and data=ue-widget', () => {

describe('when two widgets are on the same element', () => {
beforeEach(() => {
document.body.innerHTML += `<div id='three' ue='widget-one widget-two' data-widget-one-x=1></div>`;
document.body.insertAdjacentHTML(
'beforeend',
`<div id='three' ue='widget-one widget-two' data-widget-one-x=1></div>`
);
});

test('does not mark the new widget', (done) => {
Expand All @@ -98,7 +101,7 @@ describe('with attr=ue and data=ue-widget', () => {

describe('when unregisted widget is appended', () => {
beforeEach(() => {
document.body.innerHTML += `<div id='three' ue='widget-three'></div>`;
document.body.insertAdjacentHTML('beforeend', `<div id='three' ue='widget-three'></div>`);
trigger(document, 'page-refreshed');
});

Expand All @@ -112,7 +115,7 @@ describe('with attr=ue and data=ue-widget', () => {

describe('when another widget is appended', () => {
beforeEach(() => {
document.body.innerHTML += `<div id='three' ue='widget-one' data-widget-one-x=1></div>`;
document.body.insertAdjacentHTML('beforeend', `<div id='three' ue='widget-one' data-widget-one-x=1></div>`);
});

test('does not mark the new widget', (done) => {
Expand Down

0 comments on commit dca5571

Please sign in to comment.