fix: merge attributes without tag and attributes with tag

[scottohara]

This PR contains a:

• bugfix
• new feature
• code refactor
• test update
• typo fix
• metadata update

Motivation / Use-Case

Fixes #405

When processing attributes, merge attributes for any tag with attributes for the specific tag:

const handlers = new Map([
  ...(options.sources.list.get("*") || new Map()),
  ...(options.sources.list.get(tagName.toLowerCase()) || new Map()),
]);

(More info: #408 (comment))

Breaking Changes

None

Additional Info

[linux-foundation-easycla]

The committers are authorized under a signed CLA.

• ✅ Scott O'Hara (64fda8d)

[alexander-akait]

Can you provide example?

[scottohara]

Additional examples added

[alexander-akait]

I think it is bug, we need fix, attribute: "data-src" should process all tags have data-src

[scottohara]

OK, so if we're considering the current behaviour as a bug, then perhaps what I would suggest is a change to how the sources-plugin.js determines the handlers for the current tag being processed.

Given a config like this:

module.exports = {
  module: {
    rules: [
      {
        test: /\.html$/i,
        loader: "html-loader",
        options: {
          sources: {
            list: [
              // All default supported tags and attributes
              "...",
              {
                attribute: "data-src",
                type: "src",
              },
            ],
          },
        },
      },
    ],
  },
};

...internally, the normalizeSourcesList() function produces a Map like this:

const result = new Map([
 ...
 ["img", new Map([
   ["src", { type: srcType }],
   ["srcset", { type: srcsetType }]
 )],
 ...
 ["*", new Map([
   ["data-src", { type: srcType }]
 ])
]);

Current behaviour

When processing a tag, the handlers are determined as:

const handlers =
  options.sources.list.get(tagName.toLowerCase()) ||
  options.sources.list.get("*");

...which means that for an <img> tag, only the defaults are used:

Map(2) {'src' => {…}, 'srcset' => {…}}

Proposed change

The proposal is to use a union of the map entries:

const handlers = new Map([
  ...options.sources.list.get(tagName.toLowerCase() ?? new Map(),
  ...options.sources.list.get("*") ?? new Map()
]);

The result is a merged map of both the attributes specific to the tag, and any attributes that apply to all tags:

Map(3) {'src' => {…}, 'srcset' => {…}, 'data-src' => {…}}

(The ?? new Map() guards against either of those being undefined.)

If that solution sounds OK to you, I am happy to modify this PR accordingly.

[alexander-akait]

Yes, let's do it

[alexander-akait]

And let's add test case

[codecov]

Codecov Report

Merging #408 (7536bfc) into master (2fca322) will decrease coverage by 0.15%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #408      +/-   ##
==========================================
- Coverage   92.12%   91.96%   -0.16%     
==========================================
  Files           7        7              
  Lines         584      585       +1     
  Branches      187      187              
==========================================
  Hits          538      538              
- Misses         37       38       +1     
  Partials        9        9              

Impacted Files	Coverage Δ
src/plugins/sources-plugin.js	98.66% <100.00%> (-1.34%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2fca322...7536bfc. Read the comment docs.

[alexander-akait]

Big thanks for the PR, fixed here #410, Map needs to be checked on size, we need do release due some regressions in previous version, so I go ahead and do it, anyway, thanks again, release will be today