Render the Grid within Angular-Testing-Library

[zewa666]

Hey there. So today I was experimenting with rendering the actual grid in jest dom tests so that I can use angular-testing-library for behavior validations.

there were some ugly parts from slickgrid like looking for document.styleSheets but I managed to work around those. Now the grid successfully rendered (confirmed via screen.debug print). sadly though even though my dataset gets applied the provided rows do not render. So I guess the issue has to do something with timing issues or having to force manual rerenders.

I thought before diving deeper into this I'd ask here whether anyone tried to do something similar.

if it turns out not to work its no biggy either since I could switch to using playwright and real browser rendering, but its an interesting challenge ;)

[ghiscoding]

I've never used angular-testing-library but in SlickGrid if you want to call a rerender, you can do that via grid.invalidate() which invalidates all rows and forces a rendering of the UI. I'm not sure if that will make any difference in your context though.

On the other hand, I use Cypress for all my SlickGrid projects but I gave a try to Playwright in my other lib multiple-select-vanilla which is used in Slickgrid-Universal and others. I don't think there's much differences in the testing results of Cypress vs Playwright, I mean they're both E2E testing tools and they query the DOM in similar fashion. There will be some slight changes in the DOM for the next major version I'm working on but it should be small on the DOM side (there will be a few things to change in the components though, see Slickgrid-Universal Roadmap 4.0 if interested)

Maybe a potential problem with unit testing tools is that Angular-Slickgrid is not a pure Angular library, it's a SlickGrid wrapper which uses native HTML, so that could potentially add challenges with some testing libraries but Cypress/Playwright should work just fine. All my Examples are tested with Cypress and it helps quite a lot to make sure I'm not breaking anything, especially with major version (which I'm working on and expect a major release in coming month).

not sure if that helped or not... feel free to ask some more

[zewa666]

the fun thing was it already rendered the 4 rows of my dataset just without values and only the delete column. so sounds like the column mappings might not be applied binding wise. invalidate is a good idea I'll try that and see whether that helps.

yeah I know that you're using Cypress extensively. was pretty straight forward adding new ones in my PRs, especially with the help of Copilot 😅

[zewa666]

ok I'm pretty close to the solution. So the issue boils down to a missing CSSOM feature of JSDOM jsdom/jsdom#992 where no ownerNode will be created, which in turn is used by slickgrid itself (createCssRules) when creating custom rules to calculate the left/right position, which than in turn is used to render the columns. Hence explaining why my columns aren't rendered.

Will get back to you with the workaround I found

[ghiscoding]

I've opened the Wikis to public, so that you could create a Wiki if you want and provide relevant info. As a side note, that createCssRules function was created by the original SlickGrid author and I'm not exactly sure why he did it that way, but it creates CSS rules at runtime dynamically.

Also the current implementation mentioned above will change in the next major (actually that's not true, I already pushed that change in previous SlickGrid version). The logic is that it will first try to create it with document.createElement('style') or use the previous code (current code in Angular-Slickgrid, with template.innerHTML = '<style type="text/css" rel="stylesheet" />') when the first option doesn't work. For example, the first option doesn't work in Salesforce and I have to support it for our team but Salesforce is so slow to adopt new technologies that I had to put back the old code alternative to make it work in Salesforce. You can see both approach in the new SlickGrid core lib on these lines. They suck big time, they don't support .append(), createElement('style'), DocumentFragment, so many good things that I had no choice but to use alternatives

https://github.com/6pac/SlickGrid/blob/f50ed2dbc30f939f01c8cee70fef52949f00bc0d/src/slick.grid.ts#L2334-L2374

BTW, I will actually need what you found because in the next major SlickGrid will be part of Slickgrid-Universal and I will need to eventually add unit test coverage for all of that. My coverage went down by over 20% at first, I managed to get it down to 14% but I didn't add any tests for SlickGrid yet (or barely). I also use Jest with jsDOM so your info will be good to have. I guess I never faced your issue because I always mock SlickGrid for all other tests

[zewa666]

oh I see yeah that makes totally sense. So this change is already available in the latest release? I'm lagging a bit behind with my current demo project.

Yeah so for me to see what is necessary to be fixed I've started modifying slick.grid.js

function getColumnCssRules(idx) {
      var i;
      if (!stylesheet) {
        var sheets = document.styleSheets;
        sheets[0].ownerNode = _style;  <<<--------------------------

the other issue that I had was that the jsdom container that's hosting the rendered component does not have a clientWidth (jsdom/jsdom#2310).

function getViewportWidth() {
      viewportW = parseFloat(utils.innerSize(_container, 'width')) || 1000;  <---------- the || 1000 is the workaround
}

// this comes from slick.core where innerSize is calculated using clientWidth as basis

But now this test:

import { render } from '@testing-library/angular';

describe('GridDemoComponent', () => {
  it('should render counter', async () => {
    await render(GridDemoComponent, {
      imports: [AppModule],
      providers: [
        {
          provide: SupabaseRepository,
          useValue: {
            async loadAll() {
              return fakeData;
            },

            async lookupData() {
              return fakeLookupData;
            }
          },
        },
      ],
    });

    const rows = document.querySelectorAll('.slick-row')!;
    expect(Array.from(rows).length).toBeGreaterThan(0);

    expect(document.querySelector("div.slick-cell.l8.r8")).toBeInTheDocument();
  });
});

properly succeeds 🥳

 PASS  src/app/grid-demo/grid-demo.component.spec.ts
  GridDemoComponent
    √ should render counter (442 ms)

Test Suites: 1 passed, 1 total
Tests:       1 passed, 1 total
Snapshots:   0 total
Time:        4.966 s
Ran all test suites related to changed files.

---

So as a recap whats needed to make this work properly in JSDOM, at least so far:

• soften the ownerNode check, but this might already work with your latest version. Have to check that out
• add a nicer workaround for clientWidth calculation or allow to more easily mock the viewport width without having to modify source-files

[ghiscoding]

Yeah so the current version of SlickGrid used by Slickgrid-Universal/Angular-Slickgrid is using core SlickGrid v4.x (which is still written in pure JS). The newer version of core SlickGrid was migrated to TypeScript (I'm the one who made that change) but I'm not using that new code yet in Slickgrid-Universal and I'm still using the old v4 of SlickGrid (pure JS) because I am planning to take v5 code (that is now in TypeScript) and move that code entirely in Slickgrid-Universal and get rid of the core SlickGrid dependency entirely, this will make Slickgrid-Universal entirely standalone and better for troubleshooting and everything but it's still a WIP... So in short current Angular-Slickgrid uses SlickGrid v4, I left a version4 branch on that repo and I still push small fixes sometime at least until my new major is out. For your createCssRules, you can see it in version4 branch of core SlickGrid here
https://github.com/6pac/SlickGrid/blob/version4/slick.grid.js#L2058-L2098

For clientWidth tests in Jest I do it with their recommended way which is to use Object.defineProperty because that is the only way to override a readonly property. I had to do that in multiple Jest unit tests, for example I do that on the document.body to fake the window with a certain size

 Object.defineProperty(targetElm, 'clientWidth', { writable: true, configurable: true, value: 150 });

There some usage of that in the Slickgrid-Universal Resizer Service here: https://github.com/ghiscoding/slickgrid-universal/blob/a053f41abc337152628d8b19213a96836605c01b/packages/common/src/services/__tests__/resizer.service.spec.ts#L284-L299

[zewa666]

ok I see. Well in that case your change won't really help as the issue is here https://github.com/6pac/SlickGrid/blob/version4/slick.grid.js#L2119 and JSDOM simply doesn't attach an ownerNode as is done so by browsers.

As for defineProperty overrides ... so the container we're talking about is

<div class="slickgrid-container slickgrid_501049 ui-widget" style="width: 100%; overflow: hidden; outline: 0px;" id="grid1">...</div>

which gets created during rendering of my component, which on the other hand though is already too late to be monkey patched via defineProp. So I'd have to come up with an alternative approach for that

[ghiscoding]

hmm have you tried with the APP_INITIALIZER which is a bit funny since I just hung up a call with a colleague talking about this subject lol. We're trying to add full CSP for our Angular app, not using Angular-Slickgrid though, and it seems to be a bitch, the APP_INITIALIZER is even too late since the the document header is already created at the time but it might be early enough for your use case though and use Object.defineProperty

[zewa666]

I havent looked properly into it but from what I understand rendering the component will create above container and somewhere down the line also call init which causes the calculation to happen. So there is no test seam available here as in my test before I stage the component the container naturally isnt created and once render is done everything already happened.

APP init would run before the component renders hence should be as well too early

So the way I see this is that the easiest would be if Slickgrid itself could provide either a config and than use the || as I did or provide to register a callback which returns the viewport width. but both seem a bit counterintuitive from the perspective of slickgrid itself. I guess I'll open an issue over at the Slickgrid 6pac repo to track this and get fixes in there for both the clientWidth and ownerNode. Would you agree with that?

[ghiscoding]

I'm not exactly sure I understand the last bit, maybe a draft PR or some code might help to understand what you have in mind. As for the 6pac/slickgrid, I'm pretty much the one who takes care of the PR reviews and releases since the migration to TypeScript changed the project quite a lot.

Is the (onAngularGridCreated) happening too late? I mean I created that event in Angular-Slickgrid and it's being triggered right after instantiating new Slick.Grid(), I mean that could be modified to include other things if you need to

Angular-Slickgrid/src/app/modules/angular-slickgrid/components/angular-slickgrid.component.ts

Lines 656 to 658 in 567f47b

	// all instances (SlickGrid, DataView & all Services)
	this._eventPubSubService.publish('onAngularGridCreated', this._angularGridInstances);
	}

[zewa666]

sry for being too vague, I'll try to prep something tonight and share more details. but in general I dont think this is an issue of angular-slickgrid or slickgrid-universal but slickgrid-core itself

[ghiscoding]

@zewa666 was that a big change that you wanted to apply to SlickGrid for your use case? Note that I'm waiting for someone's else PR to be updated in Slickgrid-Universal before cutting a new release anyway, and I'm not sure when that will happen but I'll probably cut a release in the upcoming weekend nonetheless, so any changes you want to do by then would be great. Cheers

I'm getting closer to start major alpha in Slickgrid-Universal and probably couple more weeks to update all downstream libs.

[zewa666]

So what I'd need within slick.grid.js (from npm slickgrid) would be the following two workarounds.

slickgrid+4.0.1.patch

--- a/node_modules/slickgrid/slick.grid.js
+++ b/node_modules/slickgrid/slick.grid.js
@@ -2085,6 +2085,8 @@ if (typeof Slick === "undefined") {
       var i;
       if (!stylesheet) {
         var sheets = document.styleSheets;
+        sheets[0].ownerNode = _style;
+
         for (i = 0; i < sheets.length; i++) {
           if ((sheets[i].ownerNode || sheets[i].owningElement) == _style) {
             stylesheet = sheets[i];
@@ -3718,7 +3720,7 @@ if (typeof Slick === "undefined") {
     }
 
     function getViewportWidth() {
-      viewportW = parseFloat(utils.innerSize(_container, 'width'));
+      viewportW = parseFloat(utils.innerSize(_container, 'width')) || 1000;
     }
 
     function resizeCanvas() {

The first to overcome the lack of ownerNode support, the second to accomodate the non-existing clientWidth calculation in jsDom.
Both approaches are not valid as a fix but merely to show what parts are causing issues.

The second one could be formalized by leveraging the defaults property viewportMinWidthPx so that the line reads
viewportW = parseFloat(utils.innerSize(_container, 'width')) || options.viewportMinWidthPx;. This way I could at least define that minWidth during test and force a proper viewport.

as for the missing ownerNode ... dunno how to approach that tbh

[ghiscoding]

hmm I'm not so sure about the getViewportWidth(), can't you just spy on that method and return 1000 in your tests? I mean 1000 seems like a large number..

as for the missing ownerNode ... dunno how to approach that tbh

did you mean that you're not sure if the proposed solution you have for ownerNode will work or is it something else?

Actually looking at the ownerNode, it seems that the only reason you're doing this is to bypass the assuming missing owner on the next few lines in the for loop. Perhaps in that case we could add a new grid option to set the style ownership or maybe just skip the if condition when detecting it's used in Node (do you know how to detect the environment node vs browser?) From SO answer, it looks like we can just check if window exist or check if process exist for Node or this article

Are you going to push a PR or did you wanted me to do it? Note that Angular-Slickgrid uses an older version of SlickGrid, you can use the version4 that I keep maintaining for that reason. I can backport the changes to the other branch

[zewa666]

I like the idea of skipping it conditionally for node environments. that wouldnt case any side-effects for browser users.

the 1000 was just an example, I would actually use options.viewportMinWidthPx as mentioned instead if that aint introducing any side-effects.

I'll create the PR for the slickgrid repo and also for the mentioned version4 release than we can move this discussion over there, where it actually belongs.

[ghiscoding]

Actually another way could be to add a devMode to the SlickGrid constructor, that's actually what I've done when migrating the SlickGrid core files to Slickgrid-Universal, I needed a way for my unit tests to work too in my next branch and that came up at the time.That was a quick way to avoid my unit tests from failing in Jest

export class SlickGrid(container, data, column, options, devMode = false) {
// ...
      if ((sheet.ownerNode || sheet.owningElement) === this._style || this.devMode) {
          this.stylesheet = sheet;
          break;
       }

However I still think the Node approach is probably better, less variables to keep, but at least that's another option to chose from :)

oh and I'm also ok with options.viewportMinWidthPx for the other issue you're having, that seems fine since it doesn't introduce any breaking change.

[zewa666]

aight there it is 6pac/SlickGrid#944
if that or a variation of it gets merged I'd follow with an additonal PR for slickgrid-universal to update the globalGridOptions types for devMode. This way the typings should also upstream to angular-slickgrid.

The final test now for angular-testing-library based tests looks like the following:

const { rerender } = await render(GridDemoComponent, {
      imports: [AppModule, AngularSlickgridModule.forRoot({
        autoResize: {
          container: 'whatever_it_is_as_long_as_its_a_valid_css_selector',
        },
        devMode: {
          containerClientWidth: 1000,  // <--------- set the fake width
          ownerNodeIndex: 0            // <--------- indicate which stylesheet is supposed to resemble the injected _style
        }
      } as any)],
      providers: [
        {
          provide: SupabaseRepository,
          useValue: {
            async loadAll() {
              return fakeData;
            },

            async lookupData() {
              return fakeLookupData;
            }
          },
        },
      ],
    });

    await rerender();
    const rows = document.querySelectorAll('.slick-row')!;
    expect(Array.from(rows).length).toBeGreaterThan(0);

    expect(document.querySelector("div.slick-cell.l8.r8")).toBeInTheDocument();

[ghiscoding]

@zewa666 hey would you mind adding a quick code sample in this Wiki Testing Patterns (maybe just copy your code above or more if you want), I believe the Wikis are public now if not then let me know. Thanks

[zewa666]

oh absolutely, much better fit. I'll remove my page and add my content there

[zewa666]

Ok updated the testing page.

and ... crap you've been right with the typings yesterday. The intersection was actually wrong in the type definition. It should have been an union. Honestly I dunno what caused the union to mess up yesterday for my sample but now, seen in my test, it clear should have been the usage of devMode?: false | { ownerNodeIndex?: number; containerClientWidth?: number; };

perhaps whenever you're next time touching these parts in Slickgrid and Slickgrid-Universal you can fix that one as well.

[ghiscoding]

haha no problem, I'll do the changes and wait for anything else to come before doing another release ;)

[ghiscoding]

@zewa666 I've pushed another version including the correct devMode interface change and another small fix. This will be the last version before the next major which I expect to ship by the end of this week :) The next major will required Angular bump to v17 and also migrating to Aurelia 2 for the other repo (better late than never :P)

Anyway, for the next major, the only thing left that I'm looking at is to switch to GitBook docs if I can get the hang of it

[ghiscoding]

@zewa666 hey so out of subject, but looking at Aurelia-Slickgrid code and I just remember that you've added an optional Header/Footer using slots, I've never managed (or spend time) trying to replicate this in Angular-Slickgrid, if you know how to do the same in here, that'd be great and I would be happy to receive another PR from you 😉

BTW, new major is published

[zewa666]

yeah passing the ng-templates as children shouldnt be too troublesome. I'll take a look at it and create a new issue to keep the discussion separated