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

[T9n] "messages" prop no longer needs to be exposed for usage in tests #10399

Open
1 of 5 tasks
maxpatiiuk opened this issue Sep 26, 2024 · 1 comment
Open
1 of 5 tasks
Assignees
Labels
1 - assigned Issues that are assigned to a sprint and a team member. blocked This issue is blocked by another issue. calcite-components Issues specific to the @esri/calcite-components package. estimate - 2 Small fix or update, may require updates to tests. needs milestone Planning workflow - pending milestone assignment, has priority and/or estimate. refactor Issues tied to code that needs to be significantly reworked.

Comments

@maxpatiiuk
Copy link
Member

Description

Blocked by #10310

Every Calcite component declares messages as an undocumented prop for usage in tests.

/**
* Made into a prop for testing purposes only
*
* @internal
*/
// eslint-disable-next-line @stencil-community/strict-mutable -- updated by t9n module
@Prop({ mutable: true }) messages: ChipMessages;

I believe the only usage in tests was this place:

getCurrentMessages = async (): Promise<MessageBundle> => {
return page.$eval(tag, (component: CalciteComponentsWithMessages) => component.messages);
};

In Lumina, exposing messages as a prop is not necessary. Instead, the test can access internal component members in the following way:

 getCurrentMessages = async (): Promise<MessageBundle> => { 
   return page.$eval(tag, (el: CalciteComponentsWithMessages) => el.manager.component.messages); 
 }; 

The manager property is the same both on the html proxy element and on the actual lit component. It has a component property for accessing the lit component (in turn, component has an el property for accessing the html proxy).
This property is not part of public typings or public documentation - it's for internal usage only.

Proposed Advantages

Slight performance improvement and bundle size reduction because messages property would no longer need to be included in the lazy-loading metadata for each component.

Which Component

All

Relevant Info

No response

Calcite package

  • @esri/calcite-components
  • @esri/calcite-components-angular
  • @esri/calcite-components-react
  • @esri/calcite-design-tokens
  • @esri/eslint-plugin-calcite-components
@maxpatiiuk maxpatiiuk added refactor Issues tied to code that needs to be significantly reworked. 0 - new New issues that need assignment. needs triage Planning workflow - pending design/dev review. labels Sep 26, 2024
@github-actions github-actions bot added the calcite-components Issues specific to the @esri/calcite-components package. label Sep 26, 2024
@jcfranco jcfranco added the blocked This issue is blocked by another issue. label Sep 27, 2024
@maxpatiiuk
Copy link
Member Author

Looks like there are also a few other props that were exposed for testing only:

eg carousel.tsx:
image

@jcfranco jcfranco self-assigned this Oct 9, 2024
@jcfranco jcfranco added 1 - assigned Issues that are assigned to a sprint and a team member. estimate - 2 Small fix or update, may require updates to tests. needs milestone Planning workflow - pending milestone assignment, has priority and/or estimate. and removed 0 - new New issues that need assignment. needs triage Planning workflow - pending design/dev review. labels Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 - assigned Issues that are assigned to a sprint and a team member. blocked This issue is blocked by another issue. calcite-components Issues specific to the @esri/calcite-components package. estimate - 2 Small fix or update, may require updates to tests. needs milestone Planning workflow - pending milestone assignment, has priority and/or estimate. refactor Issues tied to code that needs to be significantly reworked.
Projects
None yet
Development

No branches or pull requests

2 participants