Skip to content

Inline inline.js #2307

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

Closed
kylecordes opened this issue Sep 23, 2016 · 12 comments
Closed

Inline inline.js #2307

kylecordes opened this issue Sep 23, 2016 · 12 comments
Labels
area: @angular-devkit/build-angular feature Issue that requests a new feature

Comments

@kylecordes
Copy link

This is a feature request as of beta.15, rest of the issue template elided.

Currently ng build --prod yields a set of files like so:

$ ls -l dist
-rw-r--r-- 1 ubuntu ubuntu    523 Sep 23 11:57 index.html
-rw-r--r-- 1 ubuntu ubuntu   1388 Sep 23 11:57 inline.js
-rw-r--r-- 1 ubuntu ubuntu 833489 Sep 23 11:57 main.62117700d901524c314e.bundle.js
-rw-r--r-- 1 ubuntu ubuntu 188979 Sep 23 11:57 main.62117700d901524c314e.bundle.js.gz
-rw-r--r-- 1 ubuntu ubuntu   3970 Sep 23 11:57 styles.b52d2076048963e7cbfd.bundle.js

Upon inspection of the the index.html and inline.js contents, it appears that the contents of inline.js were probably intended, at some point in the design or implementation process, to be inlined inside index.html, saving one HTTP round-trip in the results. Thus, a feature request to inline the inline.

Related to this, a question: it appears the styles could be included in the main bundle - is there a reason they need to be kept separate (costing another HTTP request) in a prod build? If not, please include in main bundle.

@filipesilva filipesilva added type: enhancement P5 The team acknowledges the request but does not plan to address it, it remains open for discussion labels Oct 3, 2016
@Igonato
Copy link

Igonato commented Oct 9, 2016

Also, it's missing hash in the name. It might change in the future and break a hosted app.

@tmansson
Copy link

I've also encountered the issue with not having a hash on inline.js. Since it contains the lazy loaded chunks filenames, it also need to be updated along with them.

This seems lika a major oversight, or am I missing something here? Now I manually add my version number to the inline.js filename, but that shouldn't be required.

filipesilva added a commit to filipesilva/angular-cli that referenced this issue Oct 26, 2016
We had CommonsChunkPlugin misconfigured to remove the chunkhash in inline.js.

Partially address angular#2307
filipesilva added a commit that referenced this issue Oct 28, 2016
We had CommonsChunkPlugin misconfigured to remove the chunkhash in inline.js.

Partially address #2307
Close #2899
@kylecordes
Copy link
Author

Here is an ugly(?) workaround, a bit of bashackery:

# Remove the following when https://github.com/angular/angular-cli/issues/2307 ships.

echo '<script type="text/javascript">' >inline.tmp
cat dist/inline.js >> inline.tmp
echo >>inline.tmp
echo '</script>' >>inline.tmp

sed -i.bak 's/<script type="text\/javascript" src="inline\.js"><\/script>/MARKER\
/g' dist/index.html

sed -i.bak -e '/MARKER/ {' -e 'r inline.tmp' -e 'd' -e '}' dist/index.html

It puts the contents of the inline JS output, in line in the index file where it appears to belong. The particular syntax bits above should work on both OSX and Linux, a common source of irritation with sed.

In addition to working around the problem right here in this item (a lack of optimization), more importantly this works around the problem in several items that link to this one: as of early December 2016 the in-line file does not get a hash in its file name and therefore does not cooperate very well with permanently caching JavaScript assets. This fixes that.

MRHarrison pushed a commit to MRHarrison/angular-cli that referenced this issue Feb 9, 2017
We had CommonsChunkPlugin misconfigured to remove the chunkhash in inline.js.

Partially address angular#2307
Close angular#2899
@kylecordes
Copy link
Author

Recent progress now emits the in-line file with a hash name. Here is an updated script to "in-line the inline".

#!/bin/bash
set -e

# Remove when https://github.com/angular/angular-cli/issues/2307 ships.

echo '<script type="text/javascript">' >inline.tmp
cat dist/inline.*.js >> inline.tmp
echo >>inline.tmp
echo '</script>' >>inline.tmp

sed -i.bak -E 's/<script type="text\/javascript" src="inline\.[0-9a-z]+\.bundle\.js"><\/script>/MARKER\
/g' dist/index.html

sed -i.bak -e '/MARKER/ {' -e 'r inline.tmp' -e 'd' -e '}' dist/index.html

rm inline.tmp

This saves one request, for what is just a tiny bit of JavaScript anyway.

@hansl hansl added feature Issue that requests a new feature and removed type: enhancement labels Jan 30, 2018
@hansl hansl unassigned Brocco Feb 1, 2018
@filipesilva filipesilva removed the P5 The team acknowledges the request but does not plan to address it, it remains open for discussion label Feb 1, 2018
@clydin
Copy link
Member

clydin commented Sep 28, 2018

This file no longer exists. Also with the advent of HTTP/2 there is little to no incentive to inlining in this scenario especially when coupled with the use of CSP.

@clydin clydin closed this as completed Sep 28, 2018
@dominique-mueller
Copy link

dominique-mueller commented Sep 29, 2018

Are you sure? The way I see it it just got renamed into runtime.js, but still exists. Is there a reason this file exists separately and is not part of the main.js bundle?

@clydin
Copy link
Member

clydin commented Sep 29, 2018

Long term caching. The file changes anytime another bundle changes.

@dominique-mueller
Copy link

But don't we have the per-bundle hash for that?

@clydin
Copy link
Member

clydin commented Sep 30, 2018

Long term caching was why the file was originally separate. Webpack would store the chunk names within the file. But it does not appear to do so anymore.

So yes. We could consolidate the runtime file within main now.

@dominique-mueller
Copy link

Even with HTTP/2 enaed, I think it would better to consolidate the runtime bundle with the main bundle. Just doesn't make sense to fetch a 8 KB file separately ...

@clydin
Copy link
Member

clydin commented Oct 1, 2018

With HTTP/2 it would not actually be fetched separately but at the same time and in parallel with the other bundles. In real world scenarios, the difference would be incredibly minor as the 8kb would need to be downloaded either way.
Either way this would be a breaking change; however, it is being actively considered.

@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area: @angular-devkit/build-angular feature Issue that requests a new feature
Projects
None yet
Development

No branches or pull requests

8 participants