Conflict between external reactive `class` prop and internal `classList` prop of `A`, `Link` and `NavLink` components

[100terres]

Describe the bug

Issue

When using a reactive class prop with the NavLink component, the active or inactive class get removed when the class updates even if the NavLink is still active. Based on this documentation, I suspect a conflict between how the class prop and classList works.

It's also possible, but dangerous, to mix class and classList. The main safe situation is when class is set to a static string (or nothing), and classList is reactive. (class could also be set to a static computed value as in class={baseClass()}, but then it should appear before any classList pseudo-attributes.) If both class and classList are reactive, you can get unexpected behavior: when the class value changes, Solid sets the entire class attribute, so will overwrite any toggles made by classList.

Proposed solution

If we look at the code of the A component (which is re-exported as Link and NavLink), we can see that we are using the classList prop. One solution to avoid this conflict, could be to stop using the classList prop and manually compose the class, because their will always be people using a reactive class prop.

Current code

https://github.com/solidjs/solid-router/blob/b725fc5bedf910bfcd129d4a18491992d2720dcb/src/components.tsx#L215-L228

Suggestion

  const className = createMemo(() => {
    const classes = [props.class];

    if(isActive()) {
      classes.push(props.inactiveClass)
    } else {
      classes.push(props.activeClass)
    }

    return classes.filter(Boolean).join(" ");
  });

  return (
    <a
      link
      {...rest}
      class={className}
      href={href() || props.href}
      state={JSON.stringify(props.state)}
      aria-current={isActive() ? "page" : undefined}
    />
  );

Your Example Website or App

CodeSandbox example

Steps to Reproduce the Bug or Issue

• Make the class prop of the NavLine reactive
• Update the class and the active and inactive classes of the NavLink get removed

Expected behavior

The active and inactive classes should not be removed when a reactive class prop is updated.

Screenshots or Videos

https://user-images.githubusercontent.com/4420524/194949660-a7e988a0-2220-430c-89c1-49d1c2618d0b.mp4

Platform

The issue is probably on all plaforms.

• OS: Linux
• Browser: Firefox
• Version: 105

Additional context

I noticed this issue when passing a NavLink component to the styled function of the solid-styled-components and using a theme that can be toggled (from dark to light mode). All the styles on the active NavLink was disappearing, because the styled function passes a reactive class prop to the NavLink.

[ryansolid]

I think this is a real problem, and admittedly something I introduced by allowing classList to exist. The proposed solution has the problem if the developer introduces a classList themselves I believe we hit this all over again. I think we should fix this but I think it probably has to work more granularly like classList so that we are applying styles independently instead of overwriting.

[100terres]

Hi @ryansolid,

I've opened this pull request to make sure that we are only using the classList to set the class to the A component. As you've suggested on discord we can achieve this by merging the external class prop in the classList. That way we'll be able to use the class or the classList.