TypeError: Cannot read properties of null (reading 'cssRules')

[tmcconechy]

The problem

Upgraded from 1.18.0 to 1.20.1 and now get this error on most tests:

[percy] Could not take DOM snapshot "ids-data-grid-new-light"
[percy] Error: Evaluation failed: TypeError: Cannot read properties of null (reading 'cssRules')
    at styleSheetsMatch (pptr://__puppeteer_evaluation_script__:177:50)
    at serializeCSSOM (pptr://__puppeteer_evaluation_script__:210:15)
    at serializeElements (pptr://__puppeteer_evaluation_script__:465:9)
    at Object.serializeDOM (pptr://__puppeteer_evaluation_script__:504:7)
    at pptr://__puppeteer_evaluation_script__:3:23

Environment

• Node version:
• @percy/cli version: 1.20.1
• Version of Percy SDK you’re using:
• If needed, a build or snapshot ID: https://github.com/infor-design/enterprise-wc/actions/runs/4315837554/jobs/7530775761#step:10:267
• OS version: Unix/Mac
• Type of shell command-line [interface]: bash

Details

Example test https://github.com/infor-design/enterprise-wc/blob/main/test/ids-data-grid/ids-data-grid-percy-test.ts

Debug logs

https://github.com/infor-design/enterprise-wc/actions/runs/4315837554/jobs/7530775761#step:10:267

Code to reproduce issue

If needed:

• clone https://github.com/infor-design/enterprise-wc
• change "@percy/cli": "1.18.0", to "@percy/cli": "1.20.1", in package.json
• npm i && npm run test

[tmcconechy]

@itsjwala this fixed the cssRules error but i still cant use the latest version it gives some unknown errors now not showing up in the log https://github.com/infor-design/enterprise-wc/actions/runs/4365577412/jobs/7637547882#step:10:29623

Just mentioning it in case you have an idea. If not i can try to get verbose logs turned on (is that possible I forget) when i have more time and make a new issue.

[samarsault]

@tmcconechy I see the visual changes are happening due to javascript being enabled. Is there any reason for enabling it? (If its Shadow DOM, then we support it since 1.19.2+) We noticed that the visual errors are a result of javascript side effects. Can you try disabling it and running the builds?

[tmcconechy]

@samarsault i disabled it on https://github.com/infor-design/enterprise-wc/pull/1165 but it a) still has a lot of errors b) the images dont look correct anymore.

We do use shadowRoot (web components). But not sure i recall why we enabled it. I think support suggested it. Still this only is a problem moving up from 1.18.0

https://github.com/infor-design/enterprise-wc/actions/runs/4375912710

[samarsault]

@tmcconechy we see some unusual diffs in your build and will take a look. Can you try using the latest version setting enableJavascript:true and disableShadowDOM:true?

[tmcconechy]

@samarsault still doing some weird things https://github.com/infor-design/enterprise-wc/actions/runs/4397781859/jobs/7716169624 https://github.com/infor-design/enterprise-wc/pull/1165

I would think we want JS and shadowRoot since these are JS components and they use web components/shadowRoot

[tmcconechy]

@samarsault should i be using the support at browser stack instead? My concern is we can no longer update and use percy so might need to look at other options before renewing.

[samarsault]

@tmcconechy For the enableJavascript: false case, I see the following classes of visual changes in your build

• Date & Time changes in tables : These are expected because javascript is no longer running in the rendering environment
• Width changes: This can happen If you handle responsive changes in your javascript. See this doc for reference.

Also, is the cypress test failure happening because of percy? If yes, what is the error message?
If you still face any issue, please reach out through support. It would help us in prioritizng this.

Edit:
For the enableJavascript: true case, we've identified an issue with shadow dom + enableJavascript: true configuration. Fixing this in #1210

[tmcconechy]

ok thanks @samarsault cant seem to get it to work with any configuration i can come up with. We use JS and and its all web components so Js off is not an option (will show nothing). Shadow root is also needed, without it will show nothing on the page. This is the last build https://github.com/infor-design/enterprise-wc/pull/1165 every time it just comes up with errors on each test i cant figure out. So will log a support ticket for now. Cheers

[samarsault]

@tmcconechy I have discussed this with my team and we're checking this issue on priority.

[tmcconechy]

Ok thanks. Let me know if i can provide anything more to you...

[samarsault]

How do I run percy builds locally in your project?

I tried npx percy exec -- npm run test:ci but it gives a window not defined error - consider using jsdom test environment. Same goes for npm run test:percy:local. Setting the jsdom variable introduces a page not defined error, which I'm guessing is because cypress was not initialized. I look at your github action but could not find any relevant step about this.

We've identified a fix and I want to test it before sharing

[samarsault]

@tmcconechy ^

[tmcconechy]

@samarsault you could run it one of two ways

npx percy exec -- npx jest -- percy

or just like this all the tests for percy are labeled with percy in the name

 npx jest -- percy

We dont use cypress at the moment its jest/jest puppetteer. I've never seen that error before. Did you do npm i? Are you on mac or windows?

[samarsault]

@tmcconechy I'm on mac. The following run is on the Sync local commit of your repo with a modification of testEnvironment: "jsdom".

Without the modification of testEnvironment: 'jsdom' it gives the following error

I tried npx percy exec -- npx jest -- percy and npx percy exec -- npx jest -- percy but the results were the same.

[tmcconechy]

Hmm when i try those it works ok. Im not 100% sure. Maybe try rm -rf node_modules && npm i again and make sure it didnt give any errors/things missing? It should be using puppetteer not jsdom for these "e2e" tests. We treat percy tests as e2e tests.

Failing that maybe if you want i can test the fix? Or you could try to test it by modifying the PR?

[tmcconechy]

And what about CSP? https://developer.mozilla.org/en-US/docs/Web/HTTP/CSP since we use that. i think the eval isnt allowed.

[tmcconechy]

Ok i think i got it. Steps

1. in one window run npx percy exec:start --debug
2. in another run npm run start
3. go to http://localhost:4300/ids-masthead/example.html
4. type this in the console

eval(await (await fetch('http://localhost:5338/percy/dom.js')).text())
console.log(PercyDOM.serialize().html)

In the case of masthead i see we need an extra guard but in one case its really wierd as its disconnected children from its parent element ect.. (datagrid). So really not seeing any easy fix there.

What exactly is this serialize going to do, i read the PR but didnt get it? Is it like expecting server side rendering to work? Like say i have an element like <ids-tag></ids-tag> only in the page. What is it doing to it?

Any way to disable it and do it as before?

If you want try the same tests on masthead you did on http://localhost:4300/ids-data-grid/example.html -> im not sure what i would fix. Looking at the stacktrace could it be walking the tree in the wrong order?

[itsjwala]

Concerning the resize what about this: https://github.com/infor-design/enterprise-wc/blob/main/.percy.json#L4-L7 i thought this would just do 2 screen shots in those sizes? Should we remove that and do this now: https://docs.percy.io/docs/capturing-responsive-dom-snapshots

The widths that are passed either in global config ( .percy.json in your case) or percySnapshot is used for asset discovery and rendering at that width in infrastructure browsers. but the dom snapshot which is taken is in the test browser, so you'll need to resize your browser before taking calling percySnapshot

capturing responsive dom snapshots is used when you have say 375 and 1280 widths where in the dom state is not same for both.

TLDR; in your case since you're using both the widths and they have different dom states, you have to follow capturing responsive dom snapshots

Please check this doc https://docs.percy.io/docs/debugging-sdks#how-percy-works for better understanding.

And what about CSP? https://developer.mozilla.org/en-US/docs/Web/HTTP/CSP since we use that. I think the eval isn't allowed.

yes, we require that our dom snapshot should work without any errors in your test browser, currently it requires eval, so better it to disable CSP when running Percy tests

What exactly is this serialize going to do, i read the PR but didnt get it? Is it like expecting server side rendering to work? Like say i have an element like only in the page. What is it doing to it?

Any way to disable it and do it as before?

I tried it with disableShadowDOM but it is still showing side effects, and we're any way gonna deprecate this flag in future since shadow dom APIs would be available widely.

so the best way forward with the current state was enableJavascript: false and not using disableShadowDOM

If you want try the same tests on masthead you did on http://localhost:4300/ids-data-grid/example.html -> im not sure what i would fix. Looking at the stacktrace could it be walking the tree in the wrong order?

we use DFS preorder traversal - ref
Ideally the order doesn't matter - what is causing an issue is the callbacks that are triggered I think(this is handled in Lit/Stencil components I believe since most customers are using these frameworks when using shadow dom with percy, maybe you can have a look at their implementation. )

[tmcconechy]

Ok Will review the components. See if we can fix it, i fixed one of 32 already. It might just be some are not working with document.create or something. But the data-grid one did not have an obvious fix.

Last Question For now:

• i don't 100% get what your saying about the sizes? So I should remove the config https://github.com/infor-design/enterprise-wc/blob/main/.percy.json#L4-L7 and just do https://docs.percy.io/docs/capturing-responsive-dom-snapshots#playwright-or-puppeteer instead? Or leave the config and add that anyways? Does it increase the snapshot count?
• can i run the tests in a way that shows the errors in the console? seems to hide the errors unless you run the debugger.

[itsjwala]

i don't 100% get what your saying about the sizes? So I should remove the config https://github.com/infor-design/enterprise-wc/blob/main/.percy.json#L4-L7 and just do https://docs.percy.io/docs/capturing-responsive-dom-snapshots#playwright-or-puppeteer instead? Or leave the config and add that anyways? Does it increase the snapshot count?

Please refer to this documentation

widths: An array of numbers, in pixels, representing the widths you'd like to capture for each snapshot; overrides global snapshot widths.

you can keep global config, but for snapshots where you need responsive dom snapshot do something like this(this will override global config) -

  await page.setViewport({ width: 375, height: 1024 }); 
  await percySnapshot(page, 'Home Page', { width: 375 });
  await page.setViewport({ width: 1280, height: 1024 }); 
  await percySnapshot(page, 'Home Page', { width: 1280 });

make sure to have in your global config

percy:
  defer-uploads: true

and it won't increase your screenshot count(billing perspective), since you're we're rendering same number of screenshots in infrastructure.

can i run the tests in a way that shows the errors in the console? seems to hide the errors unless you run the debugger.

The issue is this error doesn't occur in the @percy/dom package so we can't catch and log it.

[tmcconechy]

@itsjwala - I cant seem to find a way to make this work with some of our components. Take for example ids-data-grid instead of ids-masthead. You will see it errors on ids-data-grid-row because that is expecting the parent to be the data grid. You can't have a row without a data grid.

But your code is serializing this in a weird way that causes the row children to be added before the parent. Is there any way to do the old way?

According to https://github.com/percy/cli/pull/1165/files#diff-02067f8a8ffa312638a2a6525baaabf925e44fc81392b24fc9de58604100be75R27 disableShadowDom might work (didnt in my tests). But this is going away?

Other thing that is weird is a lot of text elements are duplicated in the snap shots (see some text duplicated/icons duplicated ect)...

Maybe we can have some options for serializing here? The old way did work pretty good for us

[samarsault]

Is all of this happening in the enable javascript false case?

I cant seem to find a way to make this work with some of our components. Take for example ids-data-grid instead of ids-masthead. You will see it errors on ids-data-grid-row because that is expecting the parent to be the data grid. You can't have a row without a data grid.

Can you share the expected and received HTML structure? Also share the snapshot link/id where this is happening

Other thing that is weird is a lot of text elements are duplicated in the snap shots (see some text duplicated/icons duplicated ect)...

Can you share the particular snapshots where this is happening?

[tmcconechy]

Here is the job with no JS https://github.com/infor-design/enterprise-wc/pull/1218 (still some errors i didnt fix / some i really cant).
But you can see some have duplicate text:

https://images.percy.io/1fedaa89ac4a6e622ec586b8cda83f59240718437bc432a4c79bb085bd8ab0d7
https://images.percy.io/a438c045d8ca0b5da18cbb50ffa4a51acaafdb8122315eca9546d9ff63106c8f

[samarsault]

@tmcconechy Thanks for your patience. We've been looking into your issues and here's what we got:

• Duplicate issue: I see the enterprise wc code relies heavily on attributeChangedCallback . When cloning nodes while serializing, this callback is triggered and causes side effects in our clone. Since this there’s no way to stifle attributeChangedCallback in the clone, we’ve added a workaround in 🐛 cloneNode ensure shallow clone #1230 in cli v1.23.0. Please upgrade to the same
• Serialization Errors. They are getting triggered in attributeChangedCallback but they fail since it expects the whole structure in place. Since we build the tree step by step this can be ignored. If you are seeing webpack errors in screenshots (in a dev environment), you can use the following percy css to hide it -

snapshot:
   percyCSS: |
      iframe#webpack-dev-server-client-overlay {
      display: none 
    }

• enableJavascript: false As Jigar had mentioned earlier (ref), you’ll have to take responsive dom snapshots. This is needed as javascript can lead to flaky builds

• CI Errors: There no exception being thrown or error message being displayed. We ran through individual test cases using a debugger and percySnapshot completes without any error. We ran the failing tests on a clean percy-puppeteer project and it worked fine. So it seems to be some coupling issue with the test suite and percy

[tmcconechy]

FYI after our call i let our ci run it and it did run into errors https://github.com/infor-design/enterprise-wc/actions/runs/5015136541/jobs/8990283101

I think when i ran it local it wasnt pointing to percy and just skipping tests, so I will have to look at this some more.

[samarsault]

@tmcconechy Can you refactor all your calls to percySnapshot to include the disableShadowDOM: true config? This would fallback to the old serialization algorithm. I checked with your tests and they are passing with this

You can try something like:

function customPercySnapshot(page, name, options) {
   return percySnapshot(page, name, { disableShadowDOM: true, ...options })
}

export default customPercySnapshot

And then import this as percySnapshot everywhere

[tmcconechy]

Thanks @samarsault gave that a try on this branch: https://github.com/infor-design/enterprise-wc/pull/1289/files#diff-0f2ba0c2388feb41970f6db0773b0507712e9a1121575effdf4bbccec2ecedde

Seems its doing some looping https://github.com/infor-design/enterprise-wc/actions/runs/5047016563/jobs/9053293027#step:10:50372

Did i get that all correct?

[samarsault]

The form e2e tests for which errors are shown pass fine locally. The range error is actually coming from ids-wizard. Just do npm start and go to http://localhost:4300/ids-wizard/example.html

Also can you try running the tests locally as well? (After the range fix)

[tmcconechy]

@samarsault i didnt run into this? Are you on branch fix-percy-updates and updated with git pull as i think we fixed this error a while ago.

[samarsault]

Yes I ran it on the latest fix-percy-updates branch. The CI should also be getting this. Can you also try running the tests locally once with the suggested changes and the range error fixed?