preact 10.5.7
To reproduce, clone the minimal repo, install dependencies, then run npm run watch to start a local development server.
You can switch the build from Preact to React by commenting out two lines in rollup.config.js to compare and constrast behaviors between the two.
alias({
entries: [
{ find: 'react', replacement: 'preact/compat' }, // comment out this line to see the React version
{ find: 'react-dom', replacement: 'preact/compat' }, // and this one
],
}),
I made a small video to compare the two in action.
I'm trying to use a simple CSS attribute selector on an input value, but I'm getting different behaviors in React and Preact.
In the example, the label should only have a red background when the input element is focused or when the input value is not an empty string. In React, this works as expected, but in Preact, the label always has a red background.
const Input = styled.input`
&:focus + label,
&:not([value='']):not(:focus) + label {
background-color: red;
}
`;
function App() {
const [inputVal, setInputVal] = react.useState('');
return (
<div>
<Input value={inputVal} onChange={(e) => setInputVal(e.target.value)} />
<label>
Background should only be red when focused or when value is not an empty
string.
</label>
</div>
);
}
render(<App />, document.getElementById('root'));
I'm used Styled Components in my example, but the issue still occurs when using plain CSS.
When I select the input element in devtools, the value shows as an empty string for both React and Preact. I'm not quite sure what the underlying issue is. I don't think it's related to the differences in how onChange and onInput work in React/Preact due to some small testing I've done. I know attributes and props get serialized differently in Preact. Could this be the issue?
npm installnom run watchExpected behavior is that React and Preact work identically and that background color of the text in the demo is red only when the input is focused or when the value of the input is not an empty string.
Highlighted text in the demo is red all the time.
If there's something essential I'm missing, feel free to let me know. And if this is some underlying or unfixable issue, would you have pointers in how to make a library that uses code like what's provided work correctly?
Ok I figured this out. Seems like React is serializing this as an attribute, while Preact is not. Here's my workaround:
onst inputRef = useRef(null);
const [inputValue, setInputValue] = useState("");
useEffect(() => {
// Preact uses a different methodology for serializing attributes vs props. React serializes everything to an attribute.
// We need to manually set the attribute to make the correct CSS work.
inputRef?.setAttribute("value", inputValue);
}, [inputValue]);
Should I close this? Or is this a bug that should be fixed?
I'd treat it as a bug. We probably need to add value to the list of properties here so that we fall into the .setAttribute call at the end.
Hmm I'd suggest this actually live in preact/compat and not in core. Reflecting the value property to the value attribute is a React-specific behavior and not how the DOM works or is intended to work (as I understand it). I recently learned that the value attribute is intended to be an initial value and not a reflection of the current value (MDN). The value property reflects the current value.
If we were to implement this, I think I'd want an exception for password inputs (and maybe others of similar sensitive nature, though can't think of any off the top of my head). Reflecting password values in plaintext to the DOM makes me uncomfortable. Don't have an explicit reason why but just worries me lol. React seems okay with it so maybe it is fine but Facebook doesn't use React for their password prompts (or at least the ones I checked) so it may not be an issue for them 🤷♀️
Personally, for this particular use case, I'd suggest just using a class (e.g. .blank) for this situation instead of inspecting the value attribute directly in CSS. Using a class would allow you to do some further validation before deciding if an input is blank. For example, if a user inputs whitespace (e.g. accidentally copies and pastes the wrong thing into the form), you can do something like:
if (value.trim() == '') {
classes.push('blank')
}
But that's just my preference 😄
Yeah, that’s probably how I’d do it as well. Unfortunately, this is coming from a React component library I’m consuming, so I don’t have a ton of say in the matter.
I don't think we should implement attribute reflection in Preact, or even in compat. It's a huge security nightmare that we really shouldn't be perpetuating. It was a mistake to implement in React, and is actively being considered for removal.
If you want to enable it using a hack:
import { options } from 'preact';
let old = options.vnode;
options.vnode = vnode => {
let props = vnode.props;
if (typeof vnode.type == 'string' && props && 'value' in props) {
props.Value = props.value;
}
if (old) old(vnode);
};
@milky2028 is the library you are using open source? I wonder if they'd consider changing to be more compatible with Preact and other VDom libraries.
I really appreciate the action on this issue! Preact is truly one of the most important projects in front end right now.
@andrewiggins It’s not open source, but it is an internal library from another AWS team, so I might be able to convince them to make some changes, which might be much easier if the attribute security issues are brought to light.
@developit Could you potentially elaborate on why this is a security issue? I found a few articles about XSS attacks in React, and it does seem like attribute reflection makes this easier. The risk of this issue is reduced in Preact because Preact does not use attribute reflection? Is leaking sensitive data through attributes also an issue?
One potential issue I know of is a CSS keylogger (https://css-tricks.com/css-keylogger/). Since React reflects the value into the attribute it makes inputs susceptible to this. It is admittedly a difficult attack to execute with some drawbacks (https://www.bram.us/2018/02/21/css-keylogger-and-why-you-shouldnt-worry-about-it/) but nevertheless does increase the attack vector for malicious code.
Some notes about the drawbacks mentioned in the above article to keep in mind:
FYI: If the input has a placeholder one can detect it being empty in CSS via the :placeholder-shown selector.
input:placeholder-shown {
border: 1px solid red;
}
If it has a required attribute then we can use the :invalid pseudo to detect if it's empty:
input:invalid {
border: 1px solid red;
}
@marvinhagemeister Yeah, I was thinking of suggesting we go the placeholder-shown route as well. Thank you!
I think I’ll probably close this if the plan is to not implement this in core or preact/compat. @developit’s fix works for me for now. Thank you all!
Most helpful comment
I don't think we should implement attribute reflection in Preact, or even in compat. It's a huge security nightmare that we really shouldn't be perpetuating. It was a mistake to implement in React, and is actively being considered for removal.
If you want to enable it using a hack: