useSlottedContext should not throw when slot is not found

[unional]

Provide a general summary of the issue here

Referring to this code:

if (!ctx.slots[slotKey]) {
  let availableSlots = new Intl.ListFormat().format(Object.keys(ctx.slots).map(p => `"${p}"`));
  let errorMessage = slot ? `Invalid slot "${slot}".` : 'A slot prop is required.';

  throw new Error(`${errorMessage} Valid slot names are ${availableSlots}.`);
}

https://github.com/adobe/react-spectrum/blob/main/packages/react-aria-components/src/utils.tsx#L171-L176

This violates Open/Close principle, so that it hinders the ability to compose and add new slots in extended components.

Using Button as example, I want to add a clear slot and use the button to clear the text in a input field:

<InputField>
  <ClearButton>
</InputField>

export function ClearButton() {
  return <StyledButton slot="clear">X</StyledButton>
}

export const StyledButtonContext = createContext<ContextValue<...>>(null)

export const StyledButton = forwardRef(props, ref) {
  ;[props, ref] = useContextProps(props, ref, ButtonContext]
  ;[props, ref] = useContextProps(props, ref, StyledButtonContext]

  return <Button ref={ref} {...props} style={{...}}/>
}

export function InputField({ children }) {
  return (
    <StyledButtonContext value={{ slots: { clear: { onPress: ... } } }}>
      {children}
    </StyledButtonContext>
  )
}

Since the props { slot: 'clear' } is eventually passed to useSlottedContext, it throws.

Workaround this requires manually picking out slot: 'clear' in the StyledButton, breaking encapsulation and OCP.

Something like:

export const StyledButton = forwardRef(props, ref) {
  const { slot, ...rest } = props
  
  let baseButtonProps = slot === 'clear' ? rest : props

  ;[props, ref] = useContextProps(baseButtonProps, ref, ButtonContext]
  ;[props, ref] = useContextProps({ slot, ...props}, ref, StyledButtonContext]

  baseButtonProps = slot === 'clear' ? rest : props
  
  return <Button ref={ref} {...baseButtonProps} style={{...}}/>
}

You can see this is not scalable as more slots are used in the system.

🤔 Expected Behavior?

return undefined

😯 Current Behavior

throw error

💁 Possible Solution

No response

🔦 Context

See above

🖥️ Steps to Reproduce

Hopefully the example above should be clear enough.

Version

1.13.0

What browsers are you seeing the problem on?

Chrome

If other, please specify.

No response

What operating system are you using?

macos

🧢 Your Company/Team

Palo Alto Networks

🕷 Tracking Issue

No response

[snowystinger]

Thanks for the issue. We've discussed some things around this before as we were implementing our Spectrum design system on top of RAC.

It's required like this so that we can signal to developers what sub components are required to make it accessible.

Our general stance is that you shouldn't use slot for your own contexts. Instead, you should have something like, "myDSSlot", this will guarantee that it never conflicts with our slot system. You can always derive one from the other inside a component as well.

[unional]

um... IMO the design shouldn't be constraint due the the needs on development. The slot prop is here and it is awkward that for some slots provided by RAC the consumer should use slot while others should use myDSSlot.

Is it possible to change that check to only occurs on dev build? It is a very common practice to strip dev only code when you publish.

[devongovett]

Silently ignoring invalid slot names is not a good idea for two reasons:

1. It's confusing for developers when things don't work and they get no error. Slot names can't be type-safe so runtime errors are the best we can do there.
2. If you used a slot name in your own context that isn't defined by React Aria, but then in the future we happened to add that same slot name to React Aria for a new feature, your component could break. Erroring on unknown slot names preserves forward compatibility.

Your workaround is the correct solution. If you don't want useContextProps to consume a slot, then don't pass it in. You can also reset the slot prop when you render a React Aria component.

function StyledButton(props, ref) {
  [props, ref] = useContextProps(props, ref, StyledButtonContext)
  return <Button {...props} slot={null} />
}

This is explicit about exactly what slot is consumed by your component, and what's passed to React Aria.

[unional]

Thanks for your explanation. I can understand your reasoning.

I think the problems comes down to Dependency Inversion. That the slot mechanism depends on a "fixed" list of permitted slots (concrete). I put "fixed" in qoutes because it is actually a map that you control and can add more slots to it. But the map registration is not opened to public. So it is "fixed" from the perspective of consuming library.

One way to address that is to expose the registration mechanism. Meaning making it a factory pattern.

Another problem needs to be solved is the uniqueness of keys, so that the external keys will not conflict with the internal one. Some key factory, symbol mapping, or a simple namespace prefix should do the trick.

Another potential thing to add is a key resolution factory. It is useful when an external key was introduced first, then an internal key is introduced and the external behavior wants to leverage the new internal behavior.

Essentially, it is solving the good old inheritance/extension problem.