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

Improve mount function type #60

Closed
nnaydenow opened this issue Oct 25, 2024 · 7 comments
Closed

Improve mount function type #60

nnaydenow opened this issue Oct 25, 2024 · 7 comments

Comments

@nnaydenow
Copy link

Hi,

Could you please enhance the template parameter description of mount command to have specific type instead of unknown?

Isn't it more correct to specific it as HTMLTemplateResult | string ?

Other possible solution for us is to provide export template result type into separate type which we can override.

Thanks!

@redfox-mx
Copy link
Owner

redfox-mx commented Oct 25, 2024

Hi @nnaydenow nice to meet you. Thank you for your PR 🚀

Hummm interesting 🤔. Is there some issue with unknown type?

I put unknown type since that's what lit render function does https://github.com/lit/lit/blob/1eb179f69d663440fd2ebd3589b6f2808d87494f/packages/lit-html/src/lit-html.ts#L2233

It was changed because lit cannot only render HTMLTemplateResult (and I add support for string). For example, you can pass a HTMLElement to it

cy.mount(MyCustomElement);
// behind the scenes 
render(MyCustomElement)

It's the same for arrays, arrays of elements, numbers, and other custom tagged templates.

@nnaydenow
Copy link
Author

Hi @redfox-mx,

There is no issue and everything works really nice!

We just want to add strict types for this function and since lit is expecting these types I thought it would be nice to add them.

We are using your mount command implementation with some overwrites and since mount options are extracted in a type I was able to enhance the options types (as it is described here: https://docs.cypress.io/api/cypress-api/custom-commands#Overwrite-Existing-Commands). Probably here could be used similar approach?

@redfox-mx
Copy link
Owner

Hi @nnaydenow sorry for the delay, lately I have had a lot of work.

Probably remove/change unknow type is not the best option since lit not only render HTMLTemplate results, but of course, developers needs some way to change or extend mount command.

I remove in #65 the build in definition and update docs, in this way you can use your custom type

declare global {
  namespace Cypress {
    interface Chainable {
      /**
       * Mount your template/component into Cypress sandbox
       * @param template
       * @param options render options for custom rendering
       */
      mount: <T>(template: HTMLTemplateResult | string): Cypress.Chainable<JQuery<HTMLElementTagNameMap[T]>> ;
    }
  }
}

or something like that, at least. What do u think?

@nnaydenow
Copy link
Author

Hi @redfox-mx,

Currently, I'm enhancing the options by extending the interface with my custom options.

import { MountLitTemplateOptions as OriginalMountLitTemplateOptions } from "cypress-ct-lit";

declare module "cypress-ct-lit" {
	interface MountLitTemplateOptions extends OriginalMountLitTemplateOptions {
		customOption: boolean;
	}
}

Not sure if it's the best way to enhance a build in type so I think that something similar to the proposed would also work for us.

@redfox-mx
Copy link
Owner

redfox-mx commented Nov 15, 2024

That's ok 👍🏼

For example in #65 if you want to extend mount options or change mout definition it can be more easy

import { mount as litMount, MountLitTemplateOptions } from 'cypress-ct-lit'
import { HTMLTemplateResult } from 'lit';
import { isTemplateResult } from 'lit/directive-helpers.js'

interface MountOptions extends MountLitTemplateOptions {
  customOption: boolean
}

function mount(template: HTMLTemplateResult, options?: MountOptions) {
  if(!isTemplateResult(template)) {
    throw Error('failed to mount template')
  }
  litMount(template, options);
}

declare global {
  namespace Cypress {
    interface Chainable {
      /**
       * Mount your template/component into Cypress sandbox
       * @param template
       * @param options render options for custom rendering
       */
      mount: typeof mount;
    }
  }
}

Cypress.Commands.add('mount', mount);

@redfox-mx
Copy link
Owner

closed by #65

@nnaydenow
Copy link
Author

Thanks!

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

Successfully merging a pull request may close this issue.

2 participants