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

Should isEmpty consider "empty" SafeString? #20795

Open
lifeart opened this issue Nov 14, 2024 · 3 comments
Open

Should isEmpty consider "empty" SafeString? #20795

lifeart opened this issue Nov 14, 2024 · 3 comments

Comments

@lifeart
Copy link
Contributor

lifeart commented Nov 14, 2024

If we use builtin isEmpty on empty SafeString instance, we getting incorrect result.

import { htmlSafe } from '@ember/template';
import { isEmpty } from '@ember/utils';

isEmpty(htmlSafe('')) === false

Wondering if we should update isEmpty to support SafeString...

Real life use case for error:

{{#if (isEmpty this.someEmptySafeString)}}
    it's not rendered, but ideally it should be
{{/if}}

Reproduction: https://limber.glimdown.com/edit?c=AYjmCsGcAIBsEsBuBTaAHATsx9kHcAoeAWzQHsMAXaAYTNLIDtlHqAzDe6AcgAFQExYsgwB6AMb1yzVtwDcRBlWgBvaAAtKxWAGUAhm1QBfaBy59kxAEYjRlS2lh778xeWVr4kAKKlKAT2gTM2IeXksbMQBXSnhYSFcCZAAPd2oAE2Q2PSjYanEnSBgACWRYWDIAdQpYdOgU%2B0Z0mDoGGWoVAgJoer9-fUMdSgx4RlBoAF4NLV0DZAAKbm4ASgUexiZfNACB5CGRscnp7V3FjcotgJWFbuhQZGovS-65-dHQeeXVW56sSiiMIxoE8%2BvNKOovAA6Bw7V7Dd6rW5GLo9AA89lITnsAD4uj9oKj1ABmbGlWBoETQcR6SDIVGiYm4no9fEqFQAYngbGg4KhIO2L0G8LGRmRzPF0EgwvGXl6Ap6AG1JBgsOJKABdVkqMq00X4iVSg4ymDnOUBRWjZWqjVa0RcvX41FWDC4x2MgBCuViQIAkty8PBwdBnrs3mN6YyURK2ZzubzINC%2BqHpXqJcyYYFIHNJdLgTAcJB4FZYKgFZaKNbNWm2TrkKm02bM7nZaaC0WS9AlRXkGqq9GVHa2PWJY7na7xYSSZ68qM888qTTUAGg%2BoyhSMBGSVHmTGudB5vzzfHEwLk0blsOJRmc0a89BW1526WrT2bQ39aptfE62KG9es4YN7vHebbFqW5Yqq%2Bfbimyg4OmmToutuBIerks57su6j3kwjbQGe7ybkyzIfrucYQgmGyMCGcJGpe4qUdRQq3gBqCGsBsqgR2XaQb2JFfrqv5pgxSY0cBbGHC2ZDUJx4GMC%2BvHVgO9qCcR4q3PSGKOM4yC4siIDAAQQA&format=glimdown

Why issue created? Because ember in-tempalte if already counting empty SafeString as false, but there is no official js way to check for empty safe string.

I think, we could workaround this case, extending SafeString class, adding length getter.

export class SafeString implements GlimmerSafeString {
  private __string: string;

  constructor(string: string) {
    this.__string = string;
  }

 /**
  getter to fix isEmpty case
*/
  get length() {
     return this.__string.length;
  }

  /**
    Get the string back to use as a string.

    @public
    @method toString
    @returns {String} The string marked as trusted
   */
  toString(): string {
    return `${this.__string}`;
  }

  /**
    Get the wrapped string as HTML to use without escaping.

    @public
    @method toHTML
    @returns {String} the trusted string, without any escaping applied
   */
  toHTML(): string {
    return this.toString();
  }
}
@lifeart lifeart closed this as completed Nov 14, 2024
@lifeart lifeart reopened this Nov 14, 2024
@kategengler
Copy link
Member

I believe we are unlikely to touch isEmpty and other similar utilities; I'd generally prefer utilities from other generic libraries.

@lifeart
Copy link
Contributor Author

lifeart commented Nov 25, 2024

@kategengler how about adding length to SafeString?

@kategengler
Copy link
Member

That seems more likely.

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

2 participants