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

Tweak index.html #657

Merged
merged 1 commit into from Nov 21, 2017
Merged

Tweak index.html #657

merged 1 commit into from Nov 21, 2017

Conversation

balloob
Copy link
Member

@balloob balloob commented Nov 20, 2017

Further optimizes the generation of index.html:

  • Have compatibility.js link to a working url
  • include fetch polyfill in compatibility.js (to fix translations in iOS 9)
  • Remove jinja2 if-tags. Always include compatibility.js and custom_elements_es5_adapter.js in ES5 index.html.
  • Minify index.html

Backend PR: home-assistant/core#10696

For the future, maybe we should combine compatibility.js and custom_elements_es5_adapter.js into 1 file?

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some files could not be reviewed due to errors:

Cannot read config file: .eslintrc.json
Cannot read config file: .eslintrc.json
Error: ENOENT: no such file or directory, open '.eslintrc.json'
Referenced from: .eslintrc
Error: Cannot read config file: .eslintrc.json
    at Object.fs.openSync (fs.js:652:18)
    at Object.fs.readFileSync (fs.js:553:33)
    at readFile (/app/node_modules/eslint/lib/config/config-file.js:64:15)
    at loadJSONConfigFile (/app/node_modules/eslint/lib/config/config-file.js:114:41)
    at loadConfigFile (/app/node_modules/eslint/lib/config/config-file.js:225:26)
    at loadFromDisk (/app/node_modules/eslint/lib/config/config-file.js:530:18)
    at load (/app/node_modules/eslint/lib/config/config-file.js:592:20)
    at configExtends.reduceRight (/app/node_modules/eslint/lib/config/config-file.js:421:36)
    at Array.reduceRight ()
    at applyExtends (/app/node_modules/eslint/lib/config/config-file.js:403:28)

Copy link

@houndci-bot houndci-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some files could not be reviewed due to errors:

Cannot read config file: .eslintrc.json
Cannot read config file: .eslintrc.json
Error: ENOENT: no such file or directory, open '.eslintrc.json'
Referenced from: .eslintrc
Error: Cannot read config file: .eslintrc.json
    at Object.fs.openSync (fs.js:652:18)
    at Object.fs.readFileSync (fs.js:553:33)
    at readFile (/app/node_modules/eslint/lib/config/config-file.js:64:15)
    at loadJSONConfigFile (/app/node_modules/eslint/lib/config/config-file.js:114:41)
    at loadConfigFile (/app/node_modules/eslint/lib/config/config-file.js:225:26)
    at loadFromDisk (/app/node_modules/eslint/lib/config/config-file.js:530:18)
    at load (/app/node_modules/eslint/lib/config/config-file.js:592:20)
    at configExtends.reduceRight (/app/node_modules/eslint/lib/config/config-file.js:421:36)
    at Array.reduceRight ()
    at applyExtends (/app/node_modules/eslint/lib/config/config-file.js:403:28)

const compatibilityPath = `/frontend_es5/compatibility-${md5(path.resolve(config.output_es5, 'compatibility.js'))}.js`;
const es5Extra = `
<script src='${compatibilityPath}'></script>
<script src='/frontend_es5/custom-elements-es5-adapter.js'></script>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does minifier inlines those 2 scripts?
It would be great if it did.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can add that. Should we inline core.js too?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are pros and cons:

On cold start it will save a round trip.
However index.html is fetched each time time to update cache, so by increasing this file we waste traffic.

Another option is to unite core+compatibility+adapter to a single file.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just realized that we can't combine custom-elements-es5-adapter.js with anything else. It will contain invalid syntax for older browsers which stops execution of the rest of the file.

@balloob
Copy link
Member Author

balloob commented Nov 21, 2017

I'm going ahead and merge this PR because the backend PR got merged. I'll open a PR with updates if any comments come in.

@balloob balloob merged commit 3d90d1d into master Nov 21, 2017
@balloob balloob deleted the index-html-part-2 branch November 21, 2017 05:52
@github-actions github-actions bot locked and limited conversation to collaborators Jul 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants