feat: add rule to get generated html tags from js #166

vonagam · 2019-09-13T04:10:25Z

Functionality from webapp-webpack-plugin which was introduced in this PR but for some reason was not ported back here.

My use case: usage with Next.js. Using html-webpack-plugin is not an option there, but with this addition it is possible to access html tags generated by the plugin for insertion into a head.

dcalhoun · 2019-12-26T15:26:52Z

@vonagam are you willing to provide a code example of adding this tags into the head with Next.js? I am attempting to utilize your fork, but have struggled to identify how I might use the array of HTML tags to insert them into Next.js' <Document> component. Is some form of dangerouslySetInnerHTML usage required?

import Document, { Html, Head, Main, NextScript } from 'next/document'
import favicon from "/images/icon.png";

class MyDocument extends Document {
  render() {
    return (
      <Html>
        { /* How might I add the tags into the Head below? */}
        <Head />
        <body>
          <Main />
          <NextScript />
        </body>
      </Html>
    )
  }
}

export default MyDocument

dcalhoun · 2019-12-26T15:27:01Z

@jantimon if I work get these tests passing, are you willing to merge this existing webapp-webpack-plugin feature? It'd be greatly appreciated. It appears to have been overlooked in fork merge back.

vonagam · 2019-12-27T03:58:20Z

Here is the example code that can be used to convert an array of favicon html tags to array of react elements without dangerouslySetInnerHTML:

import htmls from './path/to/favicon.svg';

const elements = htmls.map( ( html, index ) => {
  const match = html.match( /^<(\S+)\s*(.+?)\s*\/?>$/ );
  const tag = match[ 1 ];
  const attributes = match[ 2 ].split( /\s(?=\S+?=")/ );

  const props = {};
  for ( const attribute of attributes ) {
    const match = attribute.match( /^(\S+?)="(.+?)"$/ );
    const key = match[ 1 ];
    const value = match[ 2 ];
    props[ key ] = value;
  }

  const element = React.createElement( tag, { key: index, ...props } );
  return element;
} );

export default elements;

By the way, tests are passing in current form (reported checks failure has nothing to do with changes in this PR).

Rebased on current head and updated tests to use snapshots.

jantimon · 2019-12-28T08:01:21Z

This feature was removed as it is not the intention of this plugin to move the favicons meta data to the client code.

However I see your case for next.js.

@brunocodutra did already a lot of work to prepare a new major version (probably version favicons-webpack-plugin 3.0) which is probably going to change a little bit how the loader works.

Merging your changes would create some merge issues - but I really think we could get it to work somehow - please give me some time to think about it :)

vonagam · 2019-12-28T08:30:47Z

Rebased on current head.

Ping me when you think it is good time to merge this PR and i'll rebase again.

jantimon · 2019-12-31T14:20:29Z

Hey @vonagam I took the time to prepare the 3.x version: #184

Right now your pull request works as follows:

FaviconsPlugin calls the Loader for "logo.svg"
the Loader generates logos and returns the binary data back to the plugin
you use another loader and hope to hit the same side effect

Wouldn't it be better in your case if it would rather be like this:

const tags = require('favicons-webpack-plugin/src/loader!./logo.svg');
// or
import * as tags from 'favicons-webpack-plugin/src/loader!./logo.svg';

which would generate the tags into a map e.g.

async function generateFavicons (content) {
  // generateFavicons code
}

module.exports.compilations = new Map();
module.exports = (content) => {
   const favicons = generateFavicons.call(this, content);
   module.exports.compilations.set(this.query, favicons);
   return favicons;
}

In addition we split the FaviconsWebpackPlugin into two parts:

FaviconsWritterWebpackPlugin (internal plugin which would only be used directly if you want to use the loader without the main FaviconsWepackPlugin just like in your next.js example)
FaviconsWebpackPlugin (same interface as before)

The FaviconsWritterWebpackPlugin would pick up all compilations from the loader and hand it over to webpack in order to write it to the disc

the FaviconsWebpackPlugin would create a child compilation (just like now) to start the loader from the plugin directly

What do you think?

vonagam · 2019-12-31T19:04:32Z

Hm... so, if i understand your idea correctly, instead of:

const logo = '...';
const options = { ... };
const plugin = new FaviconsWebpackPlugin( { logo, ...options } );
const rule = plugin.rule();
const config = {
  plugins: [ plugin ],
  rules: [ rule ],
};

One will be able to write:

const logo = '...';
const options = { ... };
const rule = { include: logo, loader: 'favicons-webpack-plugin/src/loader', options };
const config = {
  rules: [ rule ],
};

If so, it can be considered more user friendly for such use cases and i would welcome such development.

But i personally will be happy either way as long as i get access to generated tags.

jantimon · 2021-02-06T07:55:53Z

The favicon dropped the loader approach for performance reasons..

vonagam · 2021-02-06T08:21:00Z

But this PR is not dependent on favicon using the loader approach.
Am i missing something?
Or is there a new way to get generated tags from js?

jantimon · 2021-02-06T08:57:40Z

Hey @vonagam

you are right - your approach is independent!

One problem is the following:

https://webpack.js.org/configuration/module/#useentry

Note that webpack needs to generate a unique module identifier from the resource and all loaders including options. It tries to do this with a JSON.stringify of the options object. This is fine in 99.9% of cases, but may be not unique if you apply the same loaders with different options to the resource and the options have some stringified values.

Do you have any idea how we could solve it without passing the plugin instance to the loader?

vonagam · 2021-02-06T09:54:03Z

There are two questions here:

First question: How to properly handle stringification if a plugin instance is present in options?

Next paragraph answers this:

It also breaks if the options object cannot be stringified (i.e. circular JSON). Because of this you can have a ident property in the options object which is used as unique identifier.

So ident property should be added to rule options. Which will contain a plugin unique counter id:

const logo = '...';
const options = { ... };
const plugin = new FaviconsWebpackPlugin( { logo, ...options } );
const rule = plugin.rule(); // contains plugin instance and ident in options
const config = {
  plugins: [ plugin ],
  rules: [ rule ],
};

Second question: How to make it work without passing a plugin instance to the loader?

For that to work you will need to allow passing said unique id to a plugin constructor:

const logo = '...';
const options = { ... };
const config = {
  plugins: [ new FaviconsWebpackPlugin( { logo, id: 'unique-id', ...options } ) ],
  rules: [ { include: logo, loader: 'favicons-webpack-plugin/src/loader', options: { id: 'unique-id' } ],
};

jantimon · 2021-02-06T12:49:37Z

A plugin is able to add loader rules to the compiler so we could do somethink like:

const faviconCompilerId = new WeakMap();

class FaviconWebpackPlugin {
   apply( compiler ) { 
     this.id = faviconCompilerId.get(compiler) || 0;
     faviconCompilerId.set(this.id + 1);
     
     // Inject rule into compiler using this.id 
     // this is untested pseudo code:
     compiler.options.module.rules.push( {
       include: logo,
       resourceQuery: (query) => query.indexOf('faviconTags') > -1, 
       loader: 'favicons-webpack-plugin/src/loader', 
       options: { id: this.id } 
     })
   }
}

it would allow sth like:

import tags from './my-logo.png?faviconTags';

the favicons-webpack-plugin/src/loader would a require a pitch loader to kick out all other loaders for the same file (if ?faviconTags is present).

there are some challenges:

how to handle multiple FaviconWebpackPlugin instances
does it work correctly with hashes?
does it work with `publicPath: 'auto'?

vonagam · 2021-02-06T14:26:37Z

how to handle multiple FaviconWebpackPlugin instances

Have not thought that anybody will have multiple plugin instances for same favicon file. But if this is something worth supporting, then maybe we can add an option, let's say tagsModule, and with it:

class FaviconWebpackPlugin {
  apply( compiler ) { 
    if (this.options.tagsName) {
      compiler.options.module.rules.push( {
        include: this.options.tagsModule,
        loader: 'favicons-webpack-plugin/src/loader', 
        options: { plugin: this, ident: this.options.tagsModule }
      })
    }
  }
}

And to import tags:

import tags from 'faviconTags'; // with options.tagsModule set to 'faviconTags'

jantimon · 2021-02-06T15:40:23Z

I just gave it a try and the loader injection works really well - I managed to have typescript support! :)

It looks like this:

import tags from 'favicons/runtime/tags';

console.log(tags);

I although have an idea how we can support multiple logos - the unique key is publicPath + prefix.

Now there is only one open question how can the loader get the tags from the plugin without creating a memory leak?

vonagam · 2021-02-06T15:48:51Z

There is a leak even when you use WeakMap instead of passing plugin in options?

jantimon · 2021-02-06T16:22:59Z

I found a way to inject it:

        const tagsFilePath = require.resolve('../runtime/tags.js');
        compiler.webpack.NormalModule.getCompilationHooks(compilation).loader.tap('FaviconsWebpackPlugin', (loaderContext, normalModule) => {
          if (normalModule.resource === tagsFilePath) {
            runtimeLoader.contextMap.set(loaderContext, faviconCompilation);
          }          
        });

This allows accessing the data inside the loader:

const loader = function () {
  const callback = this.async();
  const faviconCompilation = contextMap.get(this);
  if (!faviconCompilation) {
    throw new Error('broken contextMap');
  }
  
  faviconCompilation.then(({tags}) => {
    callback(null, `export default ${JSON.stringify(tags)}`);
  });
}

I created also an example inside the exampes/runtime folder:

import tags from 'favicons-webpack-plugin/runtime/tags';

console.log(tags);

which compiles to:

(() => {
  'use strict';
  console.log([
    '<link rel="shortcut icon" href="favicon.ico">',
    '<link rel="icon" type="image/png" sizes="16x16" href="favicon-16x16.png">',
    '<link rel="icon" type="image/png" sizes="32x32" href="favicon-32x32.png">',
    '<link rel="icon" type="image/png" sizes="48x48" href="favicon-48x48.png">',
    '<link rel="manifest" href="manifest.json">',
    '<meta name="mobile-web-app-capable" content="yes">',
    '<meta name="theme-color" content="#fff">',
    '<meta name="application-name" content="webapp-example">'
    // ....
      ]);
})();

jantimon · 2021-02-06T16:24:02Z

All changes have been pushed to your branch so please take a look :)

Open points:

hashes won't work
prefix is missing
supports only a single plugin instance for now
loader config is added multiple times if multiple plugin instances exist
changing the manifest.json will not trigger a new build

vonagam · 2021-02-07T07:27:36Z

hashes won't work

What exactly does it mean?

prefix is missing

Yeah, i see that you manually prepend path to resulted tags. Same should be done here too. (Should not be hard to do with regexps.)

supports only a single plugin instance for now

It is possible to use query param to distinguish them (either some id that person will pass in or something about publicPath + prefix that you mentioned). Without a query param a first compilation will be used.

loader config is added multiple times if multiple plugin instances exist

Since moduleRuleConfig is a simple frozen object, maybe before push check if it is already present in module.rules.

changing the manifest.json will not trigger a new build

Ideally addDependency(manifestPath) should be used if it is possible to get a path of a generated manifest. (If it isn't possible and this is important then there is an option to add cacheable(false) to loader.)

jantimon · 2021-02-07T10:42:17Z

hashes are placeholders which allow you to use [contenthash] in prefix to allow longterm caching

I added a new commit which:

appends the publicPath (with hash support)
adds support for multiple plugins

Typings work too (even in .js files without writing any typings):

Right now two things are missing:

addDependency for the logo and manifest file
unit tests & and intensive manual testing :)

jantimon · 2021-02-07T13:27:00Z

Okay I added dependency tracking to the loader :)

I also adjusted the api a little bit as 99% of the users will probably have only 1 favicons plugin running.

import {tags} from 'favicons-webpack-plugin/runtime/tags';

console.log(tags);

import {tagsPerPublicPath} from 'favicons-webpack-plugin/runtime/tags';

console.log(tagsPerPublicPath);

so all we need now is intensive testing

jantimon · 2021-02-09T07:55:43Z

Webpack decided to not deprecate _compilation any longer so we can simplify our code a gain for better performance..

@vonagam did you already have time to test the solution?

vonagam · 2021-02-09T07:59:31Z

Nope, sorry, bussy with other stuff...

jantimon · 2021-02-09T09:04:52Z

No worries :)
There is no rush for this feature just let me know once you had time to look into it

jantimon · 2021-02-22T06:31:02Z

@vonagam I guess this feature isn't needed anymore - can we close this?

vonagam · 2021-02-22T06:40:21Z

It is needed, but i at the moment work on non webpack realted things...
And other people want it too (since thumb ups).
But if you want to clean PRs list - feel free to close, it is yours repo.

I assume that i will work with webpack again in a month or so.
We can reopen then.

But, as i understand, everything is already done by you and you just want this to be tested in a field?
Why not merge then?

dcalhoun · 2021-02-22T12:33:43Z

For what it's worth, I would still like to see this feature merged. I appreciate your work on it @vonagam.

vonagam · 2021-10-07T15:18:09Z

I was doing non-js stuff longer than i anticipated...

Anyway, rebased PR on main branch.

The only thing that i changed was to add require.resolve for moduleRuleConfig.use (i use newer yarn and it is very strict about requiring stuff that you did not mention in deps, so if somebody to wrap favicons-webpack-plugin in their own package then favicons-webpack-plugin/src/runtime-loader.js would not resolve for end-user).

Run lint:fix and prettify:fix.
Everything seems to be working as expected.

stale · 2022-04-27T22:16:15Z

This issue had no activity for at least half a year. It's subject to automatic issue closing if there is no activity in the next 15 days.

vonagam force-pushed the feat-rule branch 4 times, most recently from aef2f1d to 9ac68f6 Compare December 27, 2019 20:39

vonagam force-pushed the feat-rule branch from 9ac68f6 to 2910839 Compare December 28, 2019 08:20

optionsome mentioned this pull request Jan 28, 2020

fix: temporarily downgrade favicons-webpack-plugin HSLdevcom/digitransit-ui#3072

Merged

jantimon closed this Feb 6, 2021

jantimon reopened this Feb 6, 2021

jantimon marked this pull request as draft February 6, 2021 16:08

jantimon force-pushed the feat-rule branch from 3885fad to 7b812e8 Compare February 6, 2021 16:18

jantimon force-pushed the feat-rule branch from a9fd9ba to 0814513 Compare February 7, 2021 13:20

Base automatically changed from master to main February 10, 2021 14:41

jantimon added 3 commits October 7, 2021 11:37

feat: make tag information accessible during runtime

34188fa

feat: make tag information accessible during runtime

f50fcc7

feat: track favicons runtime loader dependencies

5f72e46

vonagam force-pushed the feat-rule branch from 0814513 to dd7001b Compare October 7, 2021 15:11

rebase

a605244

vonagam force-pushed the feat-rule branch from dd7001b to a605244 Compare October 19, 2021 11:18

fix: prefixing should not modify original tag object

61722ea

stale bot added the wontfix label Apr 27, 2022

andy128k force-pushed the main branch 2 times, most recently from 9a9b961 to 58ee4d9 Compare October 2, 2022 02:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add rule to get generated html tags from js #166

feat: add rule to get generated html tags from js #166

vonagam commented Sep 13, 2019

dcalhoun commented Dec 26, 2019

dcalhoun commented Dec 26, 2019 •

edited

vonagam commented Dec 27, 2019

jantimon commented Dec 28, 2019

vonagam commented Dec 28, 2019

jantimon commented Dec 31, 2019 •

edited

vonagam commented Dec 31, 2019

jantimon commented Feb 6, 2021

vonagam commented Feb 6, 2021

jantimon commented Feb 6, 2021

vonagam commented Feb 6, 2021

jantimon commented Feb 6, 2021

vonagam commented Feb 6, 2021

jantimon commented Feb 6, 2021

vonagam commented Feb 6, 2021

jantimon commented Feb 6, 2021

jantimon commented Feb 6, 2021 •

edited

vonagam commented Feb 7, 2021

jantimon commented Feb 7, 2021

jantimon commented Feb 7, 2021

jantimon commented Feb 9, 2021

vonagam commented Feb 9, 2021

jantimon commented Feb 9, 2021

jantimon commented Feb 22, 2021

vonagam commented Feb 22, 2021

dcalhoun commented Feb 22, 2021

vonagam commented Oct 7, 2021

stale bot commented Apr 27, 2022

feat: add rule to get generated html tags from js #166

Are you sure you want to change the base?

feat: add rule to get generated html tags from js #166

Conversation

vonagam commented Sep 13, 2019

dcalhoun commented Dec 26, 2019

dcalhoun commented Dec 26, 2019 • edited

vonagam commented Dec 27, 2019

jantimon commented Dec 28, 2019

vonagam commented Dec 28, 2019

jantimon commented Dec 31, 2019 • edited

vonagam commented Dec 31, 2019

jantimon commented Feb 6, 2021

vonagam commented Feb 6, 2021

jantimon commented Feb 6, 2021

vonagam commented Feb 6, 2021

jantimon commented Feb 6, 2021

vonagam commented Feb 6, 2021

jantimon commented Feb 6, 2021

vonagam commented Feb 6, 2021

jantimon commented Feb 6, 2021

jantimon commented Feb 6, 2021 • edited

vonagam commented Feb 7, 2021

jantimon commented Feb 7, 2021

jantimon commented Feb 7, 2021

jantimon commented Feb 9, 2021

vonagam commented Feb 9, 2021

jantimon commented Feb 9, 2021

jantimon commented Feb 22, 2021

vonagam commented Feb 22, 2021

dcalhoun commented Feb 22, 2021

vonagam commented Oct 7, 2021

stale bot commented Apr 27, 2022

dcalhoun commented Dec 26, 2019 •

edited

jantimon commented Dec 31, 2019 •

edited

jantimon commented Feb 6, 2021 •

edited