blockly-samples icon indicating copy to clipboard operation
blockly-samples copied to clipboard

Fix usage of jsdom/jsdom-global in plugin tests

Open BenHenning opened this issue 7 months ago • 2 comments

Check for duplicates

  • [x] I have searched for similar issues before opening a new one.

Component

All plugins.

Problem

#2525 was needed due to a specific set of circumstances:

  • jsdom updated to introduce sufficient SVGElement support for our testing (I think technically it's had this for a number of years, but there was recent work that made it better). See: https://github.com/jsdom/jsdom/issues/2734.
  • jsdom-global doesn't support SVGElement, and it seems unlikely to given the lack of updates: https://github.com/rstacruz/jsdom-global/issues/60.

Request

The main issue here is that jsdom-global is unlikely to be updated to include SVGElement, and jsdom's best practices (https://github.com/jsdom/jsdom/wiki/Don't-stuff-jsdom-globals-onto-the-Node-global) suggests that we shouldn't be using it, anyway. I'm not certain on the alternative aside from prefixing window and other top-level components on all DOM references (which is likely not tenable for us).

This issue is requesting that we look into alternatives is jsdom provides to not be viable without jsdom-global. Most likely we should consider migrating the Node.js tests over to webdriver, but there may well be other options possible here.

Alternatives considered

The alternative is that we continue to work around the SVG issue and others that come up in the future.

Additional context

This may help with #2527 if the decision is to move toward webdriver tests in genreal.

BenHenning avatar May 15 '25 18:05 BenHenning

NB: The current workaround for the SVGElement reference issue is to add the following in setup (after jsdom-global initializes):

global.SVGElement = window.SVGElement;

BenHenning avatar May 15 '25 18:05 BenHenning

From a comment by @maribethb I've updated https://github.com/jsdom/jsdom/issues/2734#issuecomment-2884586480. It seems possible per the wiki page linked above that we could wrap our injection process in a jsdom context which should theoretically ensure its globals are correctly populated. This obviates the need entirely for both the workaround and jsdom-global.

While webdriver tests are still nice to add, avoiding a mass migration of plugins tests would be the best approach, I think. :)

BenHenning avatar May 15 '25 18:05 BenHenning