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

[rollup-plugin-html] extractAssets from custom tagName #1558

Open
gdbaldw opened this issue Jul 21, 2021 · 4 comments
Open

[rollup-plugin-html] extractAssets from custom tagName #1558

gdbaldw opened this issue Jul 21, 2021 · 4 comments

Comments

@gdbaldw
Copy link

gdbaldw commented Jul 21, 2021

Currently, extractAssets = boolean. But, sometimes we'd like to extract more assets, as has been identified at modernweb-dev/rocket#162 and at #1008

So, I wrote a pull request #1541 such that extractAssets = boolean | tagAndAttribute[]

Example Usage

const html = require('../../dist/index').default;

module.exports = {
  input: 'demo/spa/index.html',
  output: {
    dir: './demo/dist',
  },
  plugins: [html({
    absoluteBaseUrl: 'http://localhost:8000',
    extractAssets: [
      {tagName: 'my-first-el', attribute: 'my-src'},
      {tagName: 'my-second-el', attribute: 'my-src'},
    ]
  })],
};
@bennypowers
Copy link
Member

adjacent: prefetching assets for non-standard script types, and finding assets in the fragment of <template> tags

@gdbaldw
Copy link
Author

gdbaldw commented Aug 4, 2021

adjacent: prefetching assets for non-standard script types, and finding assets in the fragment of <template> tags

What would extractAssets interface look like? Add to the current pull request? (it's already kind of bloated, no?) I would appreciate guidance in moving this forward.

@bennypowers
Copy link
Member

I don't think you have to change much here. If you really want, add a test case for assets in template fragments

it('extracts assets in template fragments', function() {
  const fixture = `
    <img src="a.png">
    <template>
      <img src="b.png">
      <template>
        <img src="c.png">
      </template>
    </template>
  `;
  // ...
  expect(assets.length).to.equal(3);
}); 

@gdbaldw
Copy link
Author

gdbaldw commented Aug 5, 2021

I don't think you have to change much here. If you really want, add a test case for assets in template fragments

Correct! Template fragments are now included, with one recursive function, see ea62b50.

function findAllElements(
  nodes: Node | Node[],
  test: (node: Node) => boolean,
  elements: DefaultTreeElement[] = [],
): DefaultTreeElement[] {
  elements.push(...findElements(nodes, test));
  const templates = findNodes(
    nodes,
    (node: DefaultTreeNode) => node.nodeName === 'template',
  ) as Element[];
  for (const template of templates) {
    elements.push(...findAllElements(getChildNodes(getTemplateContent(template)), test));
  }
  return elements;
}

export function findAssets(document: Document, extractAssets?: boolean | TagAndAttribute[]) {
  function isAssetInjected(node: Node) {
    return isAsset(node, extractAssets);
  }
  return findAllElements(document, isAssetInjected);

Ready for code review?

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

No branches or pull requests

2 participants