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

Debug util that would only print displayed text #1135

Closed
pierrezimmermannbam opened this issue Sep 23, 2022 · 7 comments
Closed

Debug util that would only print displayed text #1135

pierrezimmermannbam opened this issue Sep 23, 2022 · 7 comments

Comments

@pierrezimmermannbam
Copy link
Collaborator

Describe the Feature

When debugging tests on large components, i.e. pages, snapshots can be very long and may not be convenient for debugging purposes. Very often I'm mostly interested in seing what text / testID is displayed on the screen and don't really care about the design so I think that a function that would log only the displayed text and maybe also the testIDs could come in pretty handy.

Possible Implementations

We'd have a function to transform the json representation into a string with just text and maybe also testIDs displayed. I implemented such a function on my current project and it looks like this :

export const toText = (node: ReactTestRendererNode | null, textSeparator = '\n'): string | null => {
  if (!node) return '';
  if (typeof node === 'string') return node;
  if (Array.isArray(node)) return toText({ type: 'Fragment', props: {}, children: node });

  const texts = [
    getImageSource(node),
    ...(node.children?.map(child => toText(child, textSeparator)) || []),
  ].filter(truthy);

  return texts.length > 0
    ? texts.join(textSeparator) +
        // Arbitrarily add a separator if more than 1 child, just to add some space in the snap
        (texts.length > 1 ? textSeparator : '')
    : null;
};

Then the api would need to be modified to add this functionality. I'm thinking of two ways to do this :

  • Add a debugText function to RenderResult (not sure about the naming yet, this could be something else)
  • Add options to the current debug function to enable customization of the output

Second option would allow more customization in the future and we may want to combine several options (for instance text and testID are visible or just text or just testID) but it also makes the api more complex and more difficult to use. Also people may not notice that this feature exists. Alternatively both approach could be combined to have an accessible and easy way of printing just text and also the ability to custom output according to one's needs.

Related Issues

No related issues as far as I know

@mdjastrzebski @AugustinLF what are your thoughts on this ?

@AugustinLF
Copy link
Collaborator

I agree with the idea of filtering output. For instance, filtering out style props would be useful for me since I never use it.

I do think we need customisable output. Because if you look at the preferred query discussion we're having (#1133), role is going to become a priority, so it'd need to be in the output. But if you don't care about it, it could be useful to skip it.

I also think we should allow debug() to take an instance (i.e. a subset of the tree). I need quite often to comment out part of the layout that just pollutes my logs...

@mdjastrzebski
Copy link
Member

@pierrezimmermannbam I think that your use case is closely related to debug function, so let's first consider integrating the narrowing down of its input.

Our current API for debug is:

function debugImpl(message?: string) {}

function debugImpl(message?: string) {

Which accepts a message: string argument.

In comparison DTL debug function is more customizable:

const debug = (
  element?: Array<Element | HTMLDocument> | Element | HTMLDocument,
  maxLength?: number,
  options?: OptionsReceived,
): void =>

https://github.com/testing-library/dom-testing-library/blob/a9a8cf26992ff0f6b4257b7300939f461d04440d/src/screen.ts#L24

And RTL return from render is:

    debug: (el = baseElement, maxLength, options) =>
      Array.isArray(el)
        ? // eslint-disable-next-line no-console
          el.forEach(e => console.log(prettyDOM(e, maxLength, options)))
        : // eslint-disable-next-line no-console,
          console.log(prettyDOM(el, maxLength, options)),

https://github.com/testing-library/react-testing-library/blob/27a9584629e28339b9961edefbb2134d7c570678/src/pure.js#L120

Quick summary:

  1. RTL/DTL debug accepts options as a 3rd argument. Most of tthese options are eventually passed to pretty-format's format function. But there is a filterNode function that allows to remove whole nodes that not match a passed predicate function
  2. RNTL accepts only single message param, tbh it does not seem very useful at the moment.
  3. RNTL also uses pretty-format's format function for our debug function

So I think that for this we should explore accepting our own options argument for debug with a custom option that would limit which component tree props to output. There are couple of ways such function could work:

  1. filterProp: (node: ReactTestRendererJSON, propName: string, propValue: unknown) => boolean - return true to include prop in output, false to ignore it. This could allow filtering out also children prop to ignore child nodes.
  2. filterProps: (node: ReactTestRendererJSON) => ReactTestRendererJSON - return a new renderer tree object, which allows the passed function to manipulate both individual props and children prop.

We could also include filterNode option as in DTL, that would allow filtering out whole nodes (without modifying their props).

@AugustinLF
Copy link
Collaborator

I love the suggested API by @mdjastrzebski. I feel filterProps should be renamed to something akin mapNode, or whatever, because here we don't filter a prop, we just rewrite what is printed.

And I think it'd be nice to also expose that as global configuration through an API akin to RTL's configure. And with of course a priority if passed as an option to debug().

@mdjastrzebski
Copy link
Member

I've submitted #1141 for adding configure

@pierrezimmermannbam
Copy link
Collaborator Author

I really like the filterProp function, and I also agree with @AugustinLF that the filterProps looks more like a map for nodes. Regarding filterProp, I think I'd like this api better though :

filterProp: (propName: string, propValue: unknown, node: ReactTestRendererJSON)

I feel like there aren't many use cases where you'd need to access the node and it also requires some knowledge on ReactTestRendererJSON type so I expect most users to use mostly propName and propValue. This would allow to declare functions that take only propName and propValue as params

Also, currently debug accepts an optional string param and I think we want to be able to add options without giving a string message, so it will either be a breaking change or we could go for the following api (for some time at least)

const debug = (options: string | DebugOptions) => {}

type DebugOptions = { 
 message?: string,
 filterProp,
...
}  

@pierrezimmermannbam
Copy link
Collaborator Author

@mdjastrzebski @AugustinLF I opened a draft pr with a first implem for the filterProps option, I'd appreciate if you could have a look and tell me what you think about it. I have some doubts regarding the implementation, namely I haven't understand why we use ReactComponent plugin in the format function as it seems to me it's never used. Also if this gets merged additional documentation on the website should be written first

@mdjastrzebski
Copy link
Member

Implemented by #1160

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

No branches or pull requests

3 participants