Skip to content

Conversation

@ajivanyandev
Copy link
Contributor

No description provided.

@ajivanyandev ajivanyandev self-assigned this Dec 28, 2025
Copilot AI review requested due to automatic review settings December 28, 2025 15:20
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a Vitest error that occurs when window.getComputedStyle is not defined during test execution. The fix adds defensive checking to prevent runtime errors in test environments where this browser API may not be available.

Key Changes:

  • Added null check for window.getComputedStyle in the readThemeMarker function to return null gracefully when the API is unavailable
  • Added comprehensive test coverage for the error handling scenario with two test cases

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
packages/devextreme/js/__internal/ui/themes.ts Added defensive check to return null when window.getComputedStyle is undefined, preventing runtime errors
packages/devextreme/testing/tests/DevExpress.ui/themes.tests.js Added new test module with two tests to verify proper handling when getComputedStyle is unavailable

Comment on lines +830 to +843
test('readThemeMarker returns null when getComputedStyle throws an error', function(assert) {
const done = assert.async();
const originalGetComputedStyle = window.getComputedStyle;
window.getComputedStyle = undefined;

try {
themes.resetTheme();
const value = themes.current();
assert.strictEqual(value, null, 'current() returns null on getComputedStyle being undefined');
} finally {
window.getComputedStyle = originalGetComputedStyle;
done();
}
});
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first test is synchronous and does not need assert.async(). The test immediately executes the try-finally block and restores the original getComputedStyle before any asynchronous operations occur. Remove the done variable and the done() call since no asynchronous operations are being performed.

Copilot uses AI. Check for mistakes.
});
});

QUnit.module('readThemeMarker error handling', () => {
Copy link

Copilot AI Dec 28, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The module definition should include a hooks parameter to allow proper cleanup of window.getComputedStyle in case tests fail. This ensures the original getComputedStyle is always restored, preventing potential interference with other tests. Consider adding a hooks.afterEach to guarantee restoration.

Suggested change
QUnit.module('readThemeMarker error handling', () => {
QUnit.module('readThemeMarker error handling', (hooks) => {
const originalGetComputedStyle = window.getComputedStyle;
hooks.afterEach(() => {
window.getComputedStyle = originalGetComputedStyle;
});

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant