Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

ebay-carousel: breaking change released as minor, not major #2401

Closed
cdaringe opened this issue Jan 23, 2025 · 2 comments
Closed

ebay-carousel: breaking change released as minor, not major #2401

cdaringe opened this issue Jan 23, 2025 · 2 comments

Comments

@cdaringe
Copy link

cdaringe commented Jan 23, 2025

Bug Report

eBayUI Version: 14.6.8

TypeError: Cannot read properties of undefined (reading 'ready') ❯ ComponentClass.onMount node_modules/@ebay/ebayui-core/dist/components/ebay-carousel/component.js:380:24 ❯ ComponentClass.___emitMount node_modules/marko/src/runtime/components/Component.js:630:26

Description

Upgrading introduced a breaking change in carousel. It appears #2352 introduced coupling to document.fonts that may not be available in all environments.

https://app.renovatebot.com/package-diff?name=@ebay%2febayui-core&from=14.5.0&to=14.6.8

Workaround

/**
 * @warn JSDOM doesn't impl document.fonts, but ebayui-core relies on it.
 */
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-expect-error
document.fonts = { ready: Promise.resolve() };

Screenshots

n/a

@agliga
Copy link
Contributor

agliga commented Jan 24, 2025

This is a widely supported browser code, especially in tier 1 and tier 2 supported browsers.
If this is a bug in jsdom, you need to add it in your jsdom mock.
https://caniuse.com/mdn-api_document_fonts

@agliga agliga closed this as completed Jan 24, 2025
@cdaringe cdaringe changed the title <component name>: <issue title> ebay-carousel: breaking change released as minor, not major Jan 29, 2025
@agliga
Copy link
Contributor

agliga commented Jan 31, 2025

To add: this is not considered breaking.
If JSDOM does not support all the default browser implementations that's a problem with JSDOM.
https://jestjs.io/docs/manual-mocks#mocking-methods-which-are-not-implemented-in-jsdom

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants