Support Server Rendering of `amp` Attribute
Closed this issue · 16 comments
Are there other attributes like this that necessitates server rendering?
Most features have corresponding DOM properties that we prefer. There's some case with app cache that I know about as well that only has server-side meaning.
If this is a more generic problem we might need a way to solve this server-side. We don't want global plugins. The client side solution is just refs and manual manipulation so we generally don't need to fix it. It is uncommon enough and doesn't create ecosystem fragmentation/isolation by global plugins (I learned something from MooTools).
@sebmarkbage Thanks for opening this issue! +1
@sebmarkbage It looks like I won't be able to use React server rendering for this. There are other attributes as well, such as amp-boilerplate that need to be included. I tried doing is amp-boilerplate, but the AMP validator then complains about the is attribute...
UPDATE:
Turns out the workaround wasn't working because it wasn't being applied to the React module actually loaded by the dependency of my project. I manage to get the following to work:
const DOMProperty = require('react/lib/ReactInjection').DOMProperty;
DOMProperty.injectDOMPropertyConfig({
Properties: {
amp: DOMProperty.MUST_USE_ATTRIBUTE,
'amp-boilerplate': DOMProperty.MUST_USE_ATTRIBUTE,
'amp-custom': DOMProperty.MUST_USE_ATTRIBUTE
},
isCustomAttribute: (attributeName) => {
return attributeName.startsWith('amp');
}
});i'm not 100% on it yet but it would seem webkit-playsinline falls into this category as well with the changes around inline & auto playing videos on iOS 10.
Inline and Auto Video Playback on iOS
When the webkit-playsinline property is specified, Safari on iPhone allows videos to play inline. Videos without the property will commence playback in fullscreen, but users can pinch close on the video to continue playing the video inline.
On iOS, videos without audio tracks or with disabled audio tracks can play automatically when the webpage loads.
@sebmarkbage Would libraries injecting features globally be more acceptable if they use a method that cannot conflict with another library?
We do have Symbols now in JS.
import amp from 'react-amp';
const Foo = () => (
React.createElement('html', {[amp]: true}/*, ... */)
);If you're worried about the situation where there are multiple copies of react, we could demand that plugins use a registration function.
import amp, {register as registerAmp} from 'react-amp';
// ...
registerAmp(React);
export default () => ReactDOM. renderToString(<Foo />);A symbol check will also allow libraries to know if they have already been registered, so registerAmp being called multiple times on the same React or multiple instances of react-amp won't be a problem either.
@dantman IMHO the first is probably a realistic path (i.e. locally provide configuration rather than register it globally), but I doubt that's the future for this feature at least. I'm not really in the loop on this, so take it with a grain of salt, but it seems the intention for unknown props is to be passed through to the node as-is in the near future. Which should solve the general problem people have.
Adding a note in this thread for the workaround above from @poscar
This workaround requires an update as of the 15.4.0 release to account for the formal react-dom split.
const DOMProperty = require('react-dom/lib/ReactInjection').DOMProperty;@pgoldrbx thanks for the advice, it was a great help.
https://github.com/Ariel-Rodriguez/react-amp-template/blob/master/src/core/index.js#L4
Despite i am almost beginner with node / SSR , i wrote this small library which facilitated very well to some partners from my work. I'd be very proud if It helped to anyone else.
Note that the code in earlier comments won’t work in React 16, but it’s also not needed because React 16 passes custom attributes through, including amp, as long as the values are strings or numbers. Booleans need to be written explicitly as strings.
I’ll close this issue but if there’s more that’s necessary specifically for AMP please file a new one with details.
Note that the code in earlier comments won’t work in React 16, but it’s also not needed because React 16 passes custom attributes through, including amp, as long as the values are strings or numbers.
This works for amp attributes that take values, but many attributes don't, and cannot to correctly pass amp validation.
Example: HTML element needs amp attribute: <html amp>. React 16 strips the attribute unless a string is passed. <html amp="true"> is not AMP valid.
@gaearon Should I open raise this in a new issue?
@misscs <html amp=""> should work though (<html amp={true}> should work too IIRC). AFAIK unless it's been changed React never renders attributes as valueless (for XML-compatibility), but valueless and empty string should be the same unless amp is unreasonably strict in its validation.
I believe <html amp="amp"> is the unabbreviated form that's valid in HTML?
@gaearon Someone complained about XML-compatibility way back and "you" changed it to never render valueless attributes. With the new renderer I guess it's less of an issue since it should only affect SSR.
Oh okay. Thanks for digging. I guess I'd prefer to drop the support for this if it's not popular, but that's definitely a breaking change and we can't do it now.