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

delete element.dataset.keyname throws TypeError: 'deleteProperty' on proxy #652

Closed
WebMechanic opened this issue Nov 8, 2022 · 4 comments · Fixed by #1038
Closed

delete element.dataset.keyname throws TypeError: 'deleteProperty' on proxy #652

WebMechanic opened this issue Nov 8, 2022 · 4 comments · Fixed by #1038
Labels
bug Something isn't working

Comments

@WebMechanic
Copy link

WebMechanic commented Nov 8, 2022

Describe the bug

To remove an attribute from an element's dataset, you should be able to use the delete operator: delete element.dataset.keyname whether .keyname exists or not.

If .keyname does not already exist on the dataset, the statement throws with a
TypeError: 'deleteProperty' on proxy: trap returned falsish for property 'keyname'.

Native DOM allows to do so and even returns true.

To Reproduce
inside a Browser with native DOM

const elt = document.body;
delete elt.dataset.fubar;  // true

inside a unit test with Happy DOM

const elt = document.body;
delete elt.dataset.fubar;     // throws
delete elt.dataset['fubar'];  // throws too

same but keys are initialised first with "anything"

const elt = document.body;
elt.dataset.foo = null;
delete elt.dataset.foo;  // works

elt.dataset.bar = undefined;
delete elt.dataset.bar;  // works too

Expected behavior
To not throw a TypeError when attempting to delete a non-existing dataset key, but silently ignore that the key does not exist.

Additional context

  • Node 16.18.0
  • Happy DOM 7.6.6
  • Vite 3.2.3
  • Vitest vitest/0.24.5

I'm writing a utility to manage multiple values inside data attributes (data-fubar="foo bar bat baz") similar to classList . Between tests I wanted to clean dataset and ran into this issue.
Not a showstopper 'cos I can just used a different key for each suite.

@WebMechanic WebMechanic added the bug Something isn't working label Nov 8, 2022
@takaya1992
Copy link
Contributor

delete operator ( deleteProperty ) is always returns true, except under exceptional conditions.

true for all cases except when the property is an own non-configurable property, in which case false is returned in non-strict mode.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/delete#return_value

but, this code returns false when the property not exist, so there is a difference in behavior.

deleteProperty: (dataset: { [key: string]: string }, key: string) => {
const name = 'data-' + DatasetUtility.camelCaseToKebab(key);
const exists = !!attributes[name];
delete attributes[name];
delete dataset[key];
return exists;
},

takaya1992 added a commit to takaya1992/happy-dom that referenced this issue Jan 6, 2023
capricorn86 added a commit that referenced this issue Jan 12, 2023
…rty-delete-when-property-not-exists

#652@patch: Fixed deleting not exist dataset properties.
@btea
Copy link
Contributor

btea commented Feb 27, 2023

@capricorn86 This issue has been fixed and should be able to be closed.

@sasha-evidation
Copy link

@btea This appears to not be fixed as of the latest version (10.11.2). Here's a very simple case that I can reproduce in a Jest test (using the Happy DOM Jest environment):

test("is able to delete a non-existent dataset key", () => {
  delete document.documentElement.dataset.xyz
})

Fails with:

TypeError: 'deleteProperty' on proxy: trap returned falsish for property 'xyz'

RussianCow added a commit to RussianCow/happy-dom that referenced this issue Aug 30, 2023
RussianCow added a commit to RussianCow/happy-dom that referenced this issue Sep 2, 2023
RussianCow added a commit to RussianCow/happy-dom that referenced this issue Sep 2, 2023
RussianCow added a commit to RussianCow/happy-dom that referenced this issue Sep 22, 2023
Add a new method, `_removeNamedItem`, which removes the item without throwing if it does not exist, and override that in subclasses instead of the primary `removeNamedItem` method.
capricorn86 added a commit that referenced this issue Sep 25, 2023
…nonexistent-key

#652@patch: Allow deletion of nonexistent keys from dataset
@capricorn86
Copy link
Owner

Thank you for reporting @WebMechanic, @takaya1992, @btea and @sasha-evidation! 🙂

Big thanks to @RussianCow for the contribution ⭐

There is a fix out now:
https://github.com/capricorn86/happy-dom/releases/tag/v12.1.7

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants