Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

🐛 Bug: Mocha adds forEach property to CSSStyleDeclaration, at least inside a Proxy #4423

Open
4 tasks done
edwinm opened this issue Aug 26, 2020 · 3 comments
Open
4 tasks done
Labels
area: browser browser-specific status: accepting prs Mocha can use your help with this one! type: bug a defect, confirmed by a maintainer

Comments

@edwinm
Copy link

edwinm commented Aug 26, 2020

Prerequisites

  • Checked that your issue hasn't already been filed by cross-referencing issues with the faq label
  • Checked next-gen ES issues and syntax problems by using the same environment and/or transpiler configuration without Mocha to ensure it isn't just a feature that actually isn't supported in the environment in question or a bug in your code.
  • 'Smoke tested' the code to be tested by running it outside the real test suite to get a better sense of whether the problem is in the code under test, your usage of Mocha, or Mocha itself
  • Ensured that there is no discrepancy between the locally and globally installed versions of Mocha. You can find them with: node node_modules/.bin/mocha --version(Local) and mocha --version(Global). We recommend that you not install Mocha globally.

Description

Since mocha 8.1.0, CSSStyleDeclaration has a forEach property, at least inside a Proxy.

This is not the case in mocha 8.0.1.

Steps to Reproduce

Checkout edwinm/carbonium@0ea6c45 :

git clone https://github.com/edwinm/carbonium.git
cd carbonium
git checkout 0ea6c45b42e6fea28720b459dbe29820ad66b5ef

Run npm i

Run npm test

All test pass

Install mocha 8.0.1: npm i @types/mocha@8.0.1 mocha@8.0.1

In src/carbonium.ts on line 146, change

-- if ("forEach" in target && !(target instanceof CSSStyleDeclaration)) {

to

++ if ("forEach" in target) {

Now my bugfix is removed.

Run npm test. All tests pass.

Install mocha 8.1.0: npm i mocha@8.1.0

Run npm test.

Actual behavior:

One test fails.

Expected behavior:

All tests pass.

Reproduces how often:

Always.

Context

Cambrium is like jQuery. It uses Proxy to merge a list of elements and element properties together.

It uses "forEach" in target to see whether we're dealing with a list of elements or element properties.

This test fails in mocha when it encouters element.style, because CSSStyleDeclaration now also has a forEach property.

I fixed this by also checking for target instanceof CSSStyleDeclaration.

It is still strange that in mocha 8.1.0, CSSStyleDeclaration suddenly has a forEach property.

This is a fringe issue and fixed on my end. The bug might still trigger problems in other (not fringe) code
and it seems like a good idea to figure out why this happens.

Versions

  • The output of npx mocha --version and node node_modules/.bin/mocha --version:
    both 8.1.0 (same bug in 8.1.2)
  • The output of node --version:
    v14.5.0
  • Your operating system
    • name and version:
      macOS Catalina 10.15.6
    • architecture (32 or 64-bit):
      64
  • Your shell (e.g., bash, zsh, PowerShell, cmd):
    zsh
  • Your browser and version (if running browser tests):
    Chrome, Firefox
  • Any third-party Mocha-related modules (and their versions):
  • Any code transpiler (e.g., TypeScript, CoffeeScript, Babel) being used (and its version):
    TypeScript 3.9.7
@outsideris
Copy link
Contributor

IMO, it caused by we introduced rollup since v8.1.0.
When I removed IE11 in .browserlistrc and then rebuild mocha.js, I can pass all tests in carbonium.
I believe mocha.js affect other Object's iteration method even I don't know why yet.

@outsideris outsideris added area: browser browser-specific type: bug a defect, confirmed by a maintainer and removed unconfirmed-bug labels Aug 30, 2020
@boneskull
Copy link
Contributor

This is surfacing via core-js and may be configurable via babel options. See zloirock/core-js#249

I'll take a further look.

@boneskull
Copy link
Contributor

This is symptomatic of a bigger issue: we are leaking polyfills into the global context.

I think I might have a fix for this... sending PR

@boneskull boneskull linked a pull request Sep 14, 2020 that will close this issue
@JoshuaKGoldberg JoshuaKGoldberg changed the title Mocha 8.1.0 adds forEach property to CSSStyleDeclaration, at least inside a Proxy 🐛 Bug: Mocha adds forEach property to CSSStyleDeclaration, at least inside a Proxy Dec 28, 2023
@JoshuaKGoldberg JoshuaKGoldberg added the status: accepting prs Mocha can use your help with this one! label Feb 6, 2024
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
area: browser browser-specific status: accepting prs Mocha can use your help with this one! type: bug a defect, confirmed by a maintainer
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants