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

Guidance about mock implementation for layout related features #2751

Open
pmdartus opened this issue Dec 14, 2019 · 6 comments
Open

Guidance about mock implementation for layout related features #2751

pmdartus opened this issue Dec 14, 2019 · 6 comments
Labels
layout Blocked on implementing a layout engine question

Comments

@pmdartus
Copy link
Member

Observations

There is a lot of interest in providing mock implementations of layout related features in jsdom especially around scroll (#1422, #2626, #2332, ...). Based on the number of reactions on the different issues, 2 of the related scroll APIs are part of the top 10 most requested features.

Goal

The goal of this issue is to capture jsdom contributors and consumers' points of view on this subject and reach a consensus on how to address such features in a coherent way.

@pmdartus
Copy link
Member Author

I think there are 2 types of layout related APIs: the APIs that can be safely mocked by either doing a no-op or returning a static value and the other ones (ok, it's really vague 🤣). jsdom already provides a pretendToBeVisual flag that can be turned ON to pretend that jsdom is rendering and displaying content. I think it would be coherent to use this flag to provide mock implementations for the simple APIs.

Scroll related APIs falls into the simple API bucket:

  • when pretendToBeVisual is false: make the APIs throw an error with a message stating that pretendToBeVisual needs to be set to true to use such API.
  • when pretendToBeVisual is true: do a no-op.

I am uncertain if we should use the same approach for APIs that jsdom already exposes (eg. window.innerWith, window.innerHeight, Element.getBoundingClientRect(), ...) because it would be a breaking change.

For other layout related APIs, that can't be simply replaced by a no-op or a static return value (eg. HTMLElement.innerText, DocumentOrShadowRoot.elementFromPoint(), IntersectionObserver), we don't expose them and require the consumer to mock such APIs.

@TimothyGu TimothyGu added the layout Blocked on implementing a layout engine label Feb 24, 2020
@mainfraame
Copy link

@pmdartus I believe a lot of people are looking for a solution to mocking those things you mentioned above, as well as others. I'm assuming that most people using jest are unknowingly introducing memory leaks into their tests by trying to mock such prototypes.

One solution that I believe would be most beneficial is if there was a callback property, similar to beforeParse that would allow you to modify the window object (including prototypes) before the actual creation of the context. Perhaps the params would be the actual window object itself, as well as the utils that include the implSymbol and symbol un-wrapper function.

@domenic
Copy link
Member

domenic commented May 22, 2021

One solution that I believe would be most beneficial is if there was a callback property, similar to beforeParse that would allow you to modify the window object (including prototypes) before the actual creation of the context.

I'm not sure what the difference is between this and beforeParse, especially in terms of what they allow you to accomplish?


Anyway, the inconsistent situation here is unfortunate. Here's one proposal, for a breaking change (to be introduced in a new major version):

  • Find a list of all such APIs. Probably most are in CSSOM View.
  • Remove all existing such mocks by default. I think those are the un-commented ones in
    partial interface Element {
    DOMRectList getClientRects();
    [NewObject] DOMRect getBoundingClientRect();
    // void scrollIntoView(optional (boolean or ScrollIntoViewOptions) arg);
    // void scroll(optional ScrollToOptions options = {});
    // void scroll(unrestricted double x, unrestricted double y);
    // void scrollTo(optional ScrollToOptions options = {});
    // void scrollTo(unrestricted double x, unrestricted double y);
    // void scrollBy(optional ScrollToOptions options = {});
    // void scrollBy(unrestricted double x, unrestricted double y);
    attribute unrestricted double scrollTop;
    attribute unrestricted double scrollLeft;
    readonly attribute long scrollWidth;
    readonly attribute long scrollHeight;
    readonly attribute long clientTop;
    readonly attribute long clientLeft;
    readonly attribute long clientWidth;
    readonly attribute long clientHeight;
    };
    but maybe there are some more?
  • Add an option, similar to today's pretendToBeVisual, call it stubLayout or something like that, which would reinstall them. Behind this flag, implement as many stubs as people ask us about. (Including all the scrolling-related ones.)
  • Document stubLayout, its limitations, and how to install your own mocks, in a dedicated readme section.

On the technical side, I'm not sure how best to accomplish this within the webidl2js infrastructure. Maybe just install them as part of normal context creation, and delete them if stubLayout is not set to true?

@TimothyGu
Copy link
Member

The webidl2js side infra sounds a bit like an ad-hoc [SecureContext]. Maybe add another extended attribute that allows feature flags?

@domenic
Copy link
Member

domenic commented May 23, 2021

Yeah, that's a good point. We might want to experiment with first introducing just [SecureContext], and using that to implement something like https://wicg.github.io/uuid/#extensions-to-the-crypto-interface , to get a more-traditional feature-flag extended attribute working.

@domenic
Copy link
Member

domenic commented May 27, 2023

To reiterate next steps for this issue: we'd like to execute the plan discussed in #2751 (comment) . The current blocker is someone making a list of all methods that could be stubbed, with links to their specs, with proposals on what their stub behavior would be. This includes ones that exist in jsdom today. And please be sure to survey closed PRs or issues to get a full sense of what people want.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
layout Blocked on implementing a layout engine question
Projects
None yet
Development

No branches or pull requests

4 participants