embroider-build / embroider

Compiling Ember apps into spec-compliant, modern Javascript.
MIT License
330 stars 137 forks source link

DOCTYPE not included in index.html while using embroider.compat build #1402

Open NagarajNN opened 1 year ago

NagarajNN commented 1 year ago

<!DOCTYPE html> not included in index.html under dist foler, which may leads to browser warning like This page is in Quirks Mode. Page layout may be impacted. For Standards Mode use “<!DOCTYPE html>” it affects style loader

NullVoxPopuli commented 1 year ago

Do you have a reproduction repo? The html used should be what's in your project, app/index,html

NagarajNN commented 1 year ago

Do you have a reproduction repo? The html used should be what's in your project, app/index,html

Here is index.html under app folder image After build project with embroider it removes Doc type element in my index.html

image

the above image shows index.html under dist folder,

It may leads to style layout loading issues when using ember serve --proxy url

NullVoxPopuli commented 1 year ago

there is a lot that is removed from that index.html, and is quite unexpected. I see missing title, meta tags, etc.

a reproduction repo would be most helpful, as the embroider apps I'm familiar with do not have this problem by default. <3

NagarajNN commented 1 year ago

.

These meta tags are inside body tag as this expected? image

Here is embroider configuration for your reference

image

NullVoxPopuli commented 1 year ago

These meta tags are inside body tag as this expected?

No -- that is very unexpected -- can you provide your index.html, or better yet a reproduction repo?

Here is embroider configuration for your reference

nothing I see in here should have the effect you're seeing. As always tho, reproducing the problem in a fresh app is a very good debugging technique.

lifeart commented 1 year ago

I think error may be related to JSDOM serialization logic, https://github.com/embroider-build/embroider/blob/a6cd4aac9b7497991b7b1acc18d0e78dccb8cd78/packages/core/src/html-entrypoint.ts#L153

ef4 commented 1 year ago

I would suggest double-checking that your input HTML file doesn't have any parse errors. JSDOM follows the error recovery part of the HTML spec, so if you have something invalid it will interpret it in a way that might surprise you.

An out-of-the-box embroider app preserves the DOCTYPE, so something else is going on here.

NagarajNN commented 1 year ago

There is no parse error in my index.html image

but the out file changed as

image

This error does not occurs in new project, i integrated embroider in my old project

Here is my pacakage.json

{ "name": "web-app", "version": "0.0.0", "private": true, "description": "Small description for dom-ser goes here", "repository": "", "license": "MIT", "author": "", "directories": { "doc": "doc", "test": "tests" }, "scripts": { "build": "ember build --environment=production", "lint": "npm-run-all --aggregate-output --continue-on-error --parallel \"lint:!(fix)\"", "lint:fix": "npm-run-all --aggregate-output --continue-on-error --parallel lint::fix", "lint:hbs": "ember-template-lint .", "lint:hbs:fix": "ember-template-lint . --fix", "lint:js": "eslint . --cache", "lint:js:fix": "eslint . --fix", "start": "ember serve", "test": "npm-run-all lint test:", "test:ember": "ember test" }, "devDependencies": { "@ember/optional-features": "^2.0.0", "@ember/test-helpers": "^2.6.0", "@embroider/compat": "^2.1.1", "@embroider/core": "^2.1.1", "@embroider/router": "^2.0.0", "@embroider/webpack": "^2.1.1", "@gfx/zopfli": "^1.0.15", "@glimmer/component": "^1.0.4", "@glimmer/tracking": "^1.0.4", "@zestia/ember-simple-infinite-scroller": "^7.0.5", "babel-eslint": "10.1.0", "broccoli-asset-rev": "^3.0.0", "compression-webpack-plugin": "^10.0.0", "css-loader": "^6.7.3", "ember-amcharts": "", "ember-echarts": "", "ember-admindroid-license":"", "css-minimizer-webpack-plugin": "^5.0.0", "ember-auto-import": "^2.4.2", "ember-classic-decorator": "^3.0.0", "ember-cli": "~3.28.5", "ember-cli-app-version": "^5.0.0", "ember-cli-babel": "^7.26.11", "ember-cli-dependency-checker": "^3.2.0", "ember-cli-flash": "^2.1.3", "ember-cli-htmlbars": "^5.7.2", "ember-cli-inject-live-reload": "^2.0.2", "ember-cli-sass": "^10.0.0", "ember-cli-shims": "^1.2.0", "ember-cli-sri": "^2.1.1", "ember-cli-terser": "^4.0.2", "ember-click-outside": "1.2.1", "ember-concurrency": "^2.2.1", "ember-data": "^3.28.6", "ember-decorators": "^6.1.1", "ember-elsewhere": "^2.0.0", "ember-export-application-global": "^2.0.1", "ember-fetch": "^8.1.1", "ember-functions-as-helper-polyfill": "^2.1.1", "ember-gridstack": "2.0.2", "ember-href-to": "3.1.0", "ember-keyboard": "^6.0.4", "ember-load-initializers": "^2.1.2", "ember-lodash": "4.19.5", "ember-maybe-import-regenerator": "^0.1.6", "ember-modifier": "^4.0.0-beta.1", "ember-moment": "^10.0.0", "ember-page-title": "^6.2.2", "ember-power-select": "^4.1.7", "ember-power-select-with-create": "^0.8.0", "ember-qunit": "^5.1.5", "ember-radio-button": "^3.0.0-beta.0", "ember-resolver": "^8.0.3", "ember-route-action-helper": "2.0.7", "ember-sortable": "^2.2.0", "ember-source": "~3.28.10", "ember-template-lint": "^3.15.0", "ember-truth-helpers": "^3.0.0", "ember-welcome-page": "^4.1.0", "eslint": "^7.32.0", "eslint-config-prettier": "^8.3.0", "eslint-plugin-ember": "^10.5.8", "eslint-plugin-ember-concurrency": "^0.5.1", "eslint-plugin-ember-es6-class": "^0.0.2", "eslint-plugin-node": "^11.1.0", "eslint-plugin-prettier": "^3.4.1", "eslint-plugin-qunit": "^6.2.0", "grapesjs": "0.20.1", "juice": "^8.0.0", "liquid-fire": "0.34.0", "loader.js": "^4.7.0", "mini-css-extract-plugin": "^2.7.5", "npm-run-all": "^4.1.5", "prettier": "^2.5.1", "qunit": "^2.17.2", "qunit-dom": "^1.6.0", "sanitize-html": "^2.7.3", "sass": "^1.17.0", "sass-loader": "^13.2.2", "terser-webpack-plugin": "^5.3.7", "webpack": "^5.78.0", "webpack-bundle-analyzer": "^4.8.0" }, "dependencies": { "ember-cli-moment-shim": "^3.8.0", "ember-in-viewport": "3.8.1", "jquery": "3.6.0", "moment": "^2.29.4" }, "engines": { "node": "12. || 14.* || >= 16" }, "ember": { "edition": "octane" } } ember-cli-build.js image

NagarajNN commented 1 year ago

When i put break point before this.dom.serialize() and before creating instance of jsdom, html fie read using "fs-extra" shows like this image

These things are happen before serialization with jsdom

I have many html files in my project but index.html only transformed llike this!

This prepareAssets() function is changing index.html like this,

in \out-2251-config_replace_config_replace_index_tree\index.html it looks normal.

image

when i removed utf8 in readFileAsync then it works fine.

html-entrypoint.js

image

ember-html.js

image

void-mAlex commented 1 year ago

@NagarajNN do you by chance have a Byte Order Mark (BOM) set on the index.html that is causing you issues?

NagarajNN commented 1 year ago

@void-mAlex this issue is fixed with changing the index.html encoding as utf-8, previously it have utf-8 with BOM. Thanks for your help! I'm closing this Now!

ef4 commented 1 year ago

I think this is a legit bug still. We parse the HTML with jsdom, and a jsdom issue about this implies that if we passed the Buffer directly to jsdom instead of first decoding it to utf-8 it would have handled this case correctly.

So here:

https://github.com/embroider-build/embroider/blob/58c9ed20e3341d667d28a6e4cac453b8a91a41f3/packages/core/src/html-entrypoint.ts#L23

We would change something like:

-this.dom = new JSDOM(readFileSync(join(this.pathToVanillaApp, this.filename), 'utf8')); 
+this.dom = new JSDOM(readFileSync(join(this.pathToVanillaApp, this.filename))); 
NullVoxPopuli commented 1 year ago

@NagarajNN how do you add a BOM?

I've applied @ef4's suggested fix here: https://github.com/embroider-build/embroider/pull/1425 but I can't seem to actually paste the BOM character. O.o (for tests)

ef4 commented 1 year ago

how do you add a BOM?

Run this in node:

fs.writeFileSync('has-bom.txt', Buffer.from('efbbbf49207374617274207769746820612062797465206f72646572206d61726b', 'hex'))

When I open the resulting file, the status line in VSCode says it's "UTF-8 with BOM".

The thing that gives it a byte order mark is the efbbbf which wikipedia tells me is the hex encoding of BOM in UTF-8.