fix(html): Template files should only include entry chunks

[ianpurvis]

Rollup Plugin Name: html

This PR contains:

• bugfix
• feature
• refactor
• documentation
• other

Are tests included?

• yes (bugfixes and features will not be merged without tests)
• no

Breaking Changes?

• yes (breaking changes will not be merged unless absolutely necessary)
• no

If yes, then include "BREAKING CHANGES:" in the first commit message body, followed by a description of what is breaking.

List any relevant issue numbers: #634

Description

I ran into a regression in #634 where non-entry scripts are injected into the default html template. This can cause unexpected behavior especially if those scripts have side-effects. This PR fixes the file filter and also adds a guard in case rollup emitted any javascript asset files.

[shellscape]

Thanks for the PR! You're on the right path, but just off of it. I think the idea is good for default behavior as well, and while it is breaking, it'll make default behavior for others easier to understand when the build gets a little bigger. So good call.

Where you want to make the change though, is here:

plugins/packages/html/src/index.ts

Lines 41 to 46 in a54d2a1

	const scripts = (files.js || [])
	.map(({ fileName }) => {
	const attrs = makeHtmlAttributes(attributes.script);
	return `<script src="${publicPath}${fileName}"${attrs}></script>`;
	})
	.join('\n');

[ianpurvis]

@shellscape Sounds good- actually, I looked at that approach too but went the other direction because I thought the behavior change was a mistake in a1b2fc3. I'll update this PR 👍

[shellscape]

I appreciate that you're trying to make this easier for the use case on your end, but I don't think you're considering the flexibility needed for the userbase as a whole. I'd like the original getFiles to be reinstated, and the block I mentioned here #688 (comment) to filter for entry files like you had originally intended. That allows any custom template or consumer to get the full stack of build files as originally intended.

[ianpurvis]

Wait, doesn't removing the filter from getFiles allow the template function to get the full stack of files? My intention in 233fe7c was to allow all files to flow to the template function, moving the isEntry filter to the default template implementation only.

About the filter itself, I am using some assumptions about Rollup's OutputAsset|OutputChunk type. Assuming that .type must equal 'chunk' or 'asset', the filter logic from a1b2fc3 already seems to be a no-op:

 (file) =>
      file.type === 'chunk' ||
      (typeof file.type === 'string' ? file.type === 'asset' : file.isAsset)

But maybe I am missing something- is that assumption invalid? Let me know, I am ready to tweak the PR as needed, just need more clarification...