mainmatter / qunit-dom

High Level DOM Assertions for QUnit
MIT License
178 stars 123 forks source link

PR 1976 was a breaking change but released as a minor update #2104

Closed lozjackson closed 4 months ago

lozjackson commented 5 months ago

https://github.com/mainmatter/qunit-dom/pull/1976 was a breaking change for us and should have been a major version update

while updating from 3.0.0 to 3.1.1 a lot of tests broke. After looking into this I found that I had camelCase property keys like this

assert.dom(selector').hasStyle({
  backgroundColor: expectedValue, // this doesn't work in 3.1.x, but worked ok in 3.0.0
});

which worked ok with 3.0.0, but started failing after updating. Turns out that the keys should be hyphenated and cameCase keys don't work any more.

assert.dom(selector').hasStyle({
  'background-color': expectedValue, // This works in both 3.0.0 and 3.1.x
});
BobrImperator commented 4 months ago

Released https://www.npmjs.com/package/qunit-dom/v/3.1.2

Feel free to re-open in case it wasn't fixed :+1:

ijlee2 commented 4 months ago

@BobrImperator I also noticed the casing issue in https://github.com/ijlee2/embroider-css-modules/pull/141/commits/bd50d626ae86b11ad4cb88f7a4faee01d5a05cea and appreciate the quick fix.

Now (or perhaps even before, I'm not sure), qunit-dom@3.1.2 allows both camel and dasherized cases, which may lead to inconsistently written code. Can you perhaps recommend at the moment which one to stick to throughout qunit-dom@v3.x?

lozjackson commented 4 months ago

@BobrImperator thanks for the quick fix

BobrImperator commented 4 months ago

@ijlee2 It appears to me that both syntaxes should be supported and people should be able to use the ones that they see fit.

To me personally it'd make more sense to stick to kebab-case even though it's less ergonomic since that's the convention and also how styles must (AFAIK) be set via css.

However the Javascript API allows you to use both camelCase and kebab-case to set and get style properties. I also don't know whether there are any caveats to kebab-case but if we were to recommend and enforce camelCase then something like variables --myVariable would suddenly become something that needs special treatement :shrug:

const element = document.querySelector("a");
const computedStyles = window.getComputedStyle(element);

computedStyle["background-color"] // "rgba(0, 0, 0, 0)"
computedStyle["backgroundColor"] // "rgba(0, 0, 0, 0)"

element.style["backgroundColor"] = "red";

computedStyle["background-color"] // "red"
computedStyle["backgroundColor"] // "red"

Considering the above, it should be decided on project-to-project basis. qunit-dom in my opinion shouldn't enforce either style and should support both meaning that bugs like this one need to be considered a regression.

ijlee2 commented 4 months ago

@BobrImperator Sounds good, thank you!