Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Polyfill without <noscript> #94

Open
vincentschroeder opened this issue Feb 9, 2021 · 15 comments
Open

Polyfill without <noscript> #94

vincentschroeder opened this issue Feb 9, 2021 · 15 comments
Assignees
Labels
question Further information is requested

Comments

@vincentschroeder
Copy link

vincentschroeder commented Feb 9, 2021

Hi, is ist somehow possible to use this polyfill without adding a <noscript> wrapper?

@mfranzke
Copy link
Owner

mfranzke commented Feb 9, 2021

@vincentschroeder thanks a lot for your question.

Sadly JavaScript seems the only possibility to hinder the images from being loaded at the very beginning - therefor it's necessary to manipulate an <img> HTML tag somehow to prevent that renderings engine downloader script from parsing that images URL and loading it upfront, but rewriting it to a regular <img> HTML tag after checking for the browsers capabilities (feature detection regarding support for the loading-attribute) and probably attaching an IntersectionObserver it case it doesn't support that loading-attribute.

Therefor manipulating that <img> HTML tag seems to be inevitable. Instead of other general solutions we've chosen to use a wrapping tag, as the other solutions that only rewrite e.g. an attribute on the <img> HTML tag wouldn't work w/o JavaScript and that for don't follow the principle of Progressive Enhancement (to the full monty).

@mfranzke mfranzke self-assigned this Feb 9, 2021
@mfranzke mfranzke added the question Further information is requested label Feb 9, 2021
@vincentschroeder
Copy link
Author

Thanks for the fast reply. I have one quick question, do I have to add the Tag a second time? Which variant is correct?

Variant 1:

<noscript class="loading-lazy">
	<img src="simpleimage.jpg" loading="lazy" alt=".." width="250" height="150" />
</noscript>

Variant 2

<noscript class="loading-lazy">
	<img src="simpleimage.jpg" loading="lazy" alt=".." width="250" height="150" />
</noscript>
<img src="simpleimage.jpg" loading="lazy" alt=".." width="250" height="150" />

@mfranzke
Copy link
Owner

mfranzke commented Feb 9, 2021

Easy. And Variant 1 is correct, compare to the source code on the demo page: https://mfranzke.github.io/loading-attribute-polyfill/demo/

@vincentschroeder
Copy link
Author

Thanks. Unfortunately, I discovered now that the polyfill is not working in SPA React applications as soon you route the noscript tag will block all images form loading.

@mfranzke
Copy link
Owner

Do you have the chance to provide an example URL, or a jsfiddle?

@mfranzke
Copy link
Owner

mfranzke commented Mar 4, 2021

@vincentschroeder I could think about to expose another method publicly that would receive some DOM node only to prepare and return it depending on browser capabilities check / feature detection, either to return it directly in case of that the browser is feasible to handle the loading-attribute natively even already, or to return it with the relevant optimizations and bindings regarding IntersectionObserver. Then anyone could easily leave out the noscript tag in case of frontend side handling of this script even only in their usecase. What do you think about that?

@cyberdelphos
Copy link

Hi @mfranzke, I have the issue when I'm using this in React, because it gives me this error: × TypeError: Cannot read property 'textContent' of null
image

This is the code I have (I've only changed the default code from React):

<div className="App">
      <header className="App-header">

      { /************** HERE ************/}
        <noscript className="loading-lazy">
          <img src={logo} className="App-logo" alt="logo" />
        </noscript>


        <p>
          Edit <code>src/App.js</code> and save to reload.
        </p>
        <a
          className="App-link"
          href="https://reactjs.org"
          target="_blank"
          rel="noopener noreferrer"
        >
          Learn React
        </a>
      </header>
    </div>

and trying to trigger the prepareElement from a useEffect hook:

useEffect(() => {
    loadingAttributePolyfill.prepareElement(document.querySelector('noscript.loading-lazy'));
  },[])

What am I doing wrong?

@mfranzke
Copy link
Owner

@cyberdelphos, thanks a lot for your feedback. I'm not that fit in react - do you have the chance to prepare a JS Bin, JSFiddle or similar test environment where I could reproduce this?

@csantos1113
Copy link

csantos1113 commented May 18, 2021

Thanks. Unfortunately, I discovered now that the polyfill is not working in SPA React applications as soon you route the noscript tag will block all images form loading.

@vincentschroeder (and probably @cyberdelphos)

You will need to write your noscript using dangerouslySetInnerHTML prop (see facebook/react#15238), something like:

<noscript
  className="loading-lazy"
  dangerouslySetInnerHTML={{
    __html: '<img alt="hello" loading="lazy" src="./hello.png" />'
  }}
/>

worth to mention: this lib works on Client side only. as soon as you import the javascript file, it'll run some lines accessing window and other browser objects that aren't be available in SSR.


Sorry for the heavy react-oriented comment in here @mfranzke - Having said that, perhaps these lines (L25:L47) could be moved into prepareElement?

@saturnonearth
Copy link

I seem to be experiencing a similar issue as Svelte blocks out anything in tags

@oyeharry
Copy link

oyeharry commented Jan 1, 2022

I was having a similar issue while integrating this polyfill with React. However, I was able to solve the issue by doing the following. It works well on SSR and CSR with React 16.8.x and should work with 17.x.x too. Thank you for the beautiful polyfill.

import React, { useEffect, useRef } from 'react';
import ReadDOMServer from 'react-dom/server';
import loadingAttributePolyfillApi from 'loading-attribute-polyfill';

export function LoadingAttrPolyfill(props) {
  const { children } = props;
  const staticMarkup = ReadDOMServer.renderToStaticMarkup(children);
  const noScriptRef = useRef(null);

  useEffect(() => {
    if (noScriptRef.current) {
      loadingAttributePolyfillApi.prepareElement(noScriptRef.current);
    }
  }, []);

  return <noscript ref={noScriptRef} dangerouslySetInnerHTML={{ __html: staticMarkup }} />;
}

export default function ImageCarousel() {
  return (
    <div>
      <LoadingAttrPolyfill>
        <img src='https://picsum.photos/200/300' alt='Random 1' />
      </LoadingAttrPolyfill>
      <LoadingAttrPolyfill>
        <picture>
          <source
            width='846'
            height='480'
            srcset='https://picsum.photos/846/480'
            media='(min-width: 768px)'
          />
          <img width='375' height='250' src='https://picsum.photos/375/250' alt='Random 2' />
        </picture>
      </LoadingAttrPolyfill>
    </div>
  );
}

@mfranzke
Copy link
Owner

mfranzke commented Jan 1, 2022

Thanks a lot @oyeharry, you‘re solution looks quite nice (from my limited knowledge about React).

@oyeharry
Copy link

oyeharry commented Jan 2, 2022

@mfranzke It seems that we will get browser global errors from the polyfill if we try to run the above code on the SSR(NodeJS). For now, as a workaround, we have to load polyfill like the following.

Will you be interested if I open a PR with some defense for browser global so that they do not execute on the server? For instance, we will only execute window, HTMLImageElement, HTMLIFrameElement, and document if we are in the browser environment or maybe run the whole script only in the browser by adding a top-level window check.

import React, { useEffect, useRef } from 'react';
import ReadDOMServer from 'react-dom/server';

let loadingAttributePolyfillApi;
if (typeof window === 'object') {
  /** 
   * import it only in the browser as this polyfill trying to execute globals
   * which are only available in the browser.
   */
  loadingAttributePolyfillApi = require('loading-attribute-polyfill').default;
}

export function LoadingAttrPolyfill(props) {
  const { children } = props;
  const staticMarkup = ReadDOMServer.renderToStaticMarkup(children);
  const noScriptRef = useRef(null);

  useEffect(() => {
    if (noScriptRef.current && loadingAttributePolyfillApi) {
      loadingAttributePolyfillApi.prepareElement(noScriptRef.current);
    }
  }, []);

  return <noscript ref={noScriptRef} dangerouslySetInnerHTML={{ __html: staticMarkup }} />;
}

export default function ImageCarousel() {
  return (
    <div>
      <LoadingAttrPolyfill>
        <img src='https://picsum.photos/200/300' alt='Random 1' />
      </LoadingAttrPolyfill>
      <LoadingAttrPolyfill>
        <picture>
          <source
            width='846'
            height='480'
            srcSet='https://picsum.photos/846/480'
            media='(min-width: 768px)'
          />
          <img width='375' height='250' src='https://picsum.photos/375/250' alt='Random 2' />
        </picture>
      </LoadingAttrPolyfill>
    </div>
  );
}

@mfranzke
Copy link
Owner

mfranzke commented Jan 2, 2022

@oyeharry sounds good to me, thanks a lot for your support !

From my point of view there‘s nothing that I remember that could be beneficial to get executed on the server, as it‘s mainly about detecting and fixing browser capabilities, for which you would need that environment.

@oyeharry
Copy link

oyeharry commented Jan 2, 2022

@mfranzke Exactly. I will open a PR soon for this update. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

6 participants