Avoid normalize calls in contains

[onx2]

Closes

✅ Pull Request Checklist:

• Included link to corresponding React Spectrum GitHub Issue.
• Added/updated unit tests and storybook for this change (for new code or code which already has tests).
• Filled out test instructions.
• Updated documentation (if it already exists for this component).
• Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

🧢 Your Project:

[onx2]

Maybe also a fast path using indexOf?

Suggested change

	if (substring.length === 0 || string.length < substring.length) {
	return true;
	}
	if (substring.length === 0 || string.length < substring.length) {
	return true;
	}
	if (string.indexOf(substring) !== -1) {
	return true;
	}

[snowystinger]

This seems like a fine optimisation. So that we know, are you running into an issue where this is slow? or it just something you noticed? or what caused you to propose the optimisation?

Edit: I read it too fast, this is going to return true for the longer substring, which I think is the opposite of what we want

Noticed code is opposite to expectation

[snowystinger]

Actually, did some more reading on normalize, two un-normalized strings can be different lengths but still be equal, so I don't think we can do this fast path at all
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/normalize In the example you could think of name2 being the substring, but if you just check the length, it's longer than string we're looking for.

Thanks for the thoughts, we can open an issue for more discussion if you don't mind still answering

So that we know, are you running into an issue where this is slow? or it just something you noticed? or what caused you to propose the optimisation?

[onx2]

This seems like a fine optimisation. So that we know, are you running into an issue where this is slow? or it just something you noticed? or what caused you to propose the optimisation?

Edit: I read it too fast, this is going to return true for the longer substring, which I think is the opposite of what we want

I wasn't running into any specific issue with it, I just noticed that it might be an optimization while implementing an fuzzyContains extension of the useFilter hook. If you found that it would cause false positives then I wouldn't advocate for it 😄

export function useFilter(options: FilterOptions = {}): Filter {
  const { threshold = 2, maxLength = 256, sensitivity = "base", ...collatorOptions } = options;
  const ariaOpts = { sensitivity, ...collatorOptions };

  const filter = useFilterAria(ariaOpts);
  const collator = useCollator(ariaOpts);

  const normalize = useMemo(
    () => (str: string) => normalizeForCollation(str, sensitivity, collator),
    [collator, sensitivity]
  );

  const fuzzyContains = useCallback(
    (text: string, query: string): boolean => {
      if (!query) return true;
      if (!text) return false;

      if (text.length > maxLength || query.length > maxLength) return false;

      if (filter.contains(text, query)) return true;

      if (Math.abs(text.length - query.length) > threshold) return false;

      const textNorm = normalize(text);
      const queryNorm = normalize(query);

      if (textNorm === queryNorm) return true;

      if (Math.abs(textNorm.length - queryNorm.length) > threshold) return false;

      return distance(textNorm, queryNorm) <= threshold;
    },
    [filter.contains, normalize, threshold, maxLength]
  );

  return {
    ...filter,
    fuzzyContains,
  };
}