webdiscus / html-bundler-webpack-plugin

Alternative to html-webpack-plugin ✅ Renders Eta, EJS, Handlebars, Nunjucks, Pug, Twig templates "out of the box" ✅ Resolves source files of scripts, styles, images in HTML ✅ Uses a template as entry point
ISC License
146 stars 14 forks source link

Resolve custom attributes, wrong type of `value` argument in filter() #36

Closed webdiscus closed 1 year ago

webdiscus commented 1 year ago

The another edge.

<a
    href="../img.png?as=webp-xl"
    data-pswp-width="990"
    data-pswp-height="1320"
    data-pswp-srcset="../img.png?as=webp-xs 540w, ../img.png?as=webp-sm 720w, ../img.png?as=webp-md 960w, ../img.png?as=webp-lg 1140w, ../img.png?as=webp-xl 1320w"
    data-cropped="true"
    target="_blank"
><img class="card-img-top" src="../img.png?as=webp-xs" alt="..."></a>

The HTML markup for Photoswipe.js. The data-pswp-srcset is simular to srcset. The queries are processed by the custom generators in image-minimizer-webpack-plugin

The plugin configuration:

...
loaderOptions: {
    sources: [
      {
        tag: 'a',
        attributes: ['href'],
        filter({ value }) {
          const path = value.split('?')[0];
          const imgRegex = new RegExp('\\.(avif|gif|heif|jp[2x]|j2[kc]|jpe?g|jpe|jxl|png|raw|tiff?|webp|svg)(?:[\\W].*)?$', 'i');

          return imgRegex.test(path)
        }
      },
      {
        tag: 'a',
        attributes: ['data-pswp-srcset'],
      },
    ],
  },
...

Now I have error: split is not a function; I'm surprised...

New config:

...
loaderOptions: {
    sources: [
      {
        tag: 'a',
        attributes: ['href'],
        filter({ value }) {
          if (typeof(value) !== 'string') {
            console.log(typeof (value), value);

            return false;
          }
          const path = value.split('?')[0];
          const imgRegex = new RegExp('\\.(avif|gif|heif|jp[2x]|j2[kc]|jpe?g|jpe|jxl|png|raw|tiff?|webp|svg)([?:\\W].*)?$', 'i');

          return imgRegex.test(path)
        }
      },
      {
        tag: 'a',
        attributes: ['data-pswp-srcset'],
      },
    ],
  },
...

Terminal:

...
object [
  '../img.png?as=webp-xs',
  '../img.png?as=webp-sm',
  '../img.png?as=webp-md ',
  '../img.png?as=webp-lg',
  '../img.png?as=webp-xl'
]
...

Object is data-pswp-srcset And now it lost the width values. I'm realy surprised...

Originally posted by @vralle in https://github.com/webdiscus/html-bundler-webpack-plugin/discussions/34#discussioncomment-7277639

vralle commented 1 year ago
...
{
        tag: 'a',
        attributes: ['href', 'data-pswp-srcset'],
        filter({ attributes }) {
          if (attributes.href) {
            const path = attributes.href.split('?')[0];
            const imgRegex = new RegExp('\\.(avif|gif|heif|jp[2x]|j2[kc]|jpe?g|jpe|jxl|png|raw|tiff?|webp|svg)$', 'i');

            return imgRegex.test(path)
          }

          if (attributes['data-pswp-srcset']) {
            return true;
          }

          return false;
        }
      },
...

This way works perfect.

Problem's with value.

webdiscus commented 1 year ago

@vralle

yes, I wanted to say that you need to add the custom (not standard) attribute name data-pswp-srcset :-)

webdiscus commented 1 year ago

@vralle

Problem's with value.

what is problem with value, the split is not a function; or another?

vralle commented 1 year ago

split not function because value has type Object instead of string for custom srcset data-pswp-srcset in first config.

vralle commented 1 year ago
loaderOptions: {
    sources: [
      {
        tag: 'img',
        attributes: ['src', 'srcset'],
        filter({ attribute, value }) {
          console.log(attribute, typeof (value));

          return true;
        }
      }
    ]
}

terminal:

src string
src string
srcset object
src string
src string
src string

types

webdiscus commented 1 year ago

@vralle

it is right, because srcset contains many parsed sources

vralle commented 1 year ago

srcset is an Object === value is not a string

webdiscus commented 1 year ago

if attribute name contains srcset substring, then value is an array containing all parsed filenames, w/o size

vralle commented 1 year ago

Can we do anything about string type of the value?)

vralle commented 1 year ago

It's type export)

webdiscus commented 1 year ago

you can check the attribute by name or by type:

         {
            tag: 'img',
            attributes: ['srcset'],
            filter: ({ tag, attribute, value, attributes, resourcePath }) => {
              // ignore srcset when containig `somefile.png`
              if (attribute.includes('srcset') && !value.find((item) => item.endsWith('somefile.png'))) return false;
            },
          },

It's type export)

ah, yo, I do it :-) I will exend the value type as string | Array<string>

vralle commented 1 year ago

Is typeof(value) wrong when it return Object for `srcset?

webdiscus commented 1 year ago

the value type of srcset is always Array<string>

webdiscus commented 1 year ago

@vralle ,

I have a question, how would be better for the filter using the srcset attribute:

<img src="image.png"  srcset="imagge1.png 200w, image2.png 400w, image3.png 600w">
  1. the value is an array containing parsed filenames (as is now):
    filter: ({attributes, attribute, value,}) => {
      const originalValue = attributes.scrset; // => 'imagge1.png 200w, image2.png 400w, image3.png 600w'
      // the `value` is ['imagge1.png', 'imagge2.png', 'imagge3.png']
    }
  2. the value contains original string, imagge1.png 200w, image2.png 400w, image3.png 600w, should be added NEW argument values as an array containing parsed filenames:
    filter: ({attributes, attribute, value, values}) => {
      // the `value` is 'imagge1.png 200w, image2.png 400w, image3.png 600w'
      // the `values` is ['imagge1.png', 'imagge2.png', 'imagge3.png']
    }

Variant 1: I must nothing do, you can get original value string via attributes.srcset. Variant 2: I need implement this, it is not complex, but it is breaking change and I can add it only in next major version.

vralle commented 1 year ago

Long version. I always look at the IDE's prompts. Based on them, I expected the value of the attributes to be of type string. Without looking into the source code of the plugin, the first thing that comes to mind is that the filter gives the name of the attributes and their raw values. I was checking multiple 'a' tag attributes in one filter. For srcset I used the attribute name. But for href I used the value. In the documentation the type is string, in the head the type is string. The plugin suggested an array. Webpack threw an error because my code expected a string and I didn't understand what exactly needed to be fixed.

Let me return to the answer to the question. I prefer it to be a string. This eliminates some confusion about value types. One type is string. To access the value after parsing, it would be convenient to have another property. For example, parsedValue

vralle commented 1 year ago

I also think that none of this is a problem. After some clarification on the value types issue, my code will check the attribute value types.

But there are still questions with SVG sprites; I have not found a way to dynamically inject CSS file to DOM (shadow dom) using js. There are probably many more corners.

webdiscus commented 1 year ago

@vralle thank you!

you are fully right! I will keep the value as original string for all attributes and add the NEW parsedValue argument for parsed values of the srcset attribute.

webdiscus commented 1 year ago

But there are still questions with SVG sprites;

please create new issue with details..

I have not found a way to dynamically inject CSS file using js

What you means?

You can import any source style file in JS, although it's bad practice:

import './myStyle.scss'; // <= import style

console.log('Hello World!');

The imported style in JS will be extracted into separate CSS with the basename of the JS file. See the usage example in the test case js-import-css-nested-sorted

webdiscus commented 1 year ago

@vralle

v2.15.0 has fixed type of value for srcset. Now it's original string. Added parsedValue as Array<string> contains parsed filenames, w/o URL query.

webdiscus commented 1 year ago

@vralle

The issue with type of value argument is done. For another question or issue please create new issue.