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

Added support to update manifest appfiles from index.html #701

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
47 changes: 47 additions & 0 deletions index.js
Expand Up @@ -346,6 +346,7 @@ module.exports = {
},

postBuild(result) {
this._updateAppFilesInManifest(result.directory);
Copy link
Member

Choose a reason for hiding this comment

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

Is there a way for us to only do this in Embroider land?

Copy link
Contributor

Choose a reason for hiding this comment

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

There's no obvious way and I don't really want to give one to people. Running Embroider and a classic build within the same broccoli system is a valid thing to want to do.

Copy link
Contributor Author

@dnalagatla dnalagatla Aug 5, 2019

Choose a reason for hiding this comment

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

@ef4 any reasons for not to provide a way to check the embroider land. It will make it easier for Fastboot build to switch between classic build flow and changes needed to be made for embroider. Also, will help in isolating the build issue specific to embroider.

Copy link
Contributor

Choose a reason for hiding this comment

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

There's really no such thing as "outside" vs "inside" embroider. The addons all get instantiated before any embroider code even runs, and that is intentional. Also, the last time we exposed a global flag like this inside broccoli (the one for checking for fastboot) we regretted it, because it's just not really how broccoli works.

So no, I really don't want v1-formatted addons to start adding code to behave special under embroider. The only supported way to do that is to ship as a native V2 addon, after we merge RFC 507.

Also, this change is an improvement for all builds, not just embroider. It moves us in a better architectural direction.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you @ef4. This makes sense.

if (this.fastboot) {
// should we reload fastboot if there are only css changes? Seems it maynot be needed.
// TODO(future): we can do a smarter reload here by running fs-tree-diff on files loaded
Expand Down Expand Up @@ -376,5 +377,51 @@ module.exports = {

_isModuleUnification() {
return (typeof this.project.isModuleUnification === 'function') && this.project.isModuleUnification();
},

_updateAppFilesInManifest(appDir) {
const pkgPath = path.join(appDir, 'package.json');
const pkg = require(pkgPath);
const {
fastboot
} = pkg;

if(fastboot) {
const { manifest: { appFiles: manifestAppFiles } } = fastboot;
let appFilePath = '';
if(Array.isArray(manifestAppFiles) && typeof manifestAppFiles[0] === 'string') {
appFilePath = path.resolve(appDir, manifestAppFiles[0]);
}
// If app file in package.json doesn't exist, update manifest.appFiles in package.json
// with appFiles defined in index.html. This check will ensure the existing behavior is intact and
// only update from index.html for embroider flow.
if(appFilePath && !existsSync(appFilePath)) {
const appFiles = this._getAppFilesFromIndexHtml(appDir);
manifestAppFiles.splice(0, 1, ...appFiles);
fs.writeFileSync(pkgPath, JSON.stringify(pkg));
}
}
},

_getAppFilesFromIndexHtml(appDir) {
const cheerio = require('cheerio');
const indexHtml = fs.readFileSync(path.join(appDir, 'index.html'));
const $ = cheerio.load(indexHtml);
const scriptFileNameRegEx = /([a-zA-Z0-9_\.\-\(\):])+(\.js)/ig;
const filesToSkipRgex = /^vendor|^vendor-static|^ember-cli-live-reload/gi;
const appFiles = [];

$('script').each(function(i, elem) {
const src = $(elem).attr('src');
if (src) {
const fileName = src.match(scriptFileNameRegEx)[0];
const filePath = path.join(appDir, 'assets', fileName);
if (fileName && existsSync(filePath) && !filesToSkipRgex.test(fileName)) {
appFiles.push(path.relative(appDir, filePath));
}
}
});

return appFiles;
}
};
1 change: 1 addition & 0 deletions package.json
Expand Up @@ -45,6 +45,7 @@
"chai": "^4.1.2",
"chai-fs": "^2.0.0",
"chai-string": "^1.4.0",
"cheerio": "^1.0.0-rc.3",
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this should be a dependency not a devDependency

Copy link
Contributor

Choose a reason for hiding this comment

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

"co": "^4.6.0",
"ember-ajax": "^3.1.0",
"ember-cli": "~3.3.0",
Expand Down
100 changes: 97 additions & 3 deletions yarn.lock
Expand Up @@ -1635,6 +1635,10 @@ body@^5.1.0:
raw-body "~1.1.0"
safe-json-parse "~1.0.1"

boolbase@~1.0.0:
version "1.0.0"
resolved "https://registry.yarnpkg.com/boolbase/-/boolbase-1.0.0.tgz#68dff5fbe60c51eb37725ea9e3ed310dcc1e776e"

bower-config@^1.3.0:
version "1.4.1"
resolved "https://registry.yarnpkg.com/bower-config/-/bower-config-1.4.1.tgz#85fd9df367c2b8dbbd0caa4c5f2bad40cd84c2cc"
Expand Down Expand Up @@ -2387,6 +2391,17 @@ check-error@^1.0.1:
version "1.0.2"
resolved "https://registry.yarnpkg.com/check-error/-/check-error-1.0.2.tgz#574d312edd88bb5dd8912e9286dd6c0aed4aac82"

cheerio@^1.0.0-rc.3:
version "1.0.0-rc.3"
resolved "https://registry.yarnpkg.com/cheerio/-/cheerio-1.0.0-rc.3.tgz#094636d425b2e9c0f4eb91a46c05630c9a1a8bf6"
dependencies:
css-select "~1.2.0"
dom-serializer "~0.1.1"
entities "~1.1.1"
htmlparser2 "^3.9.1"
lodash "^4.15.0"
parse5 "^3.0.1"

chownr@^1.0.1:
version "1.0.1"
resolved "https://registry.yarnpkg.com/chownr/-/chownr-1.0.1.tgz#e2a75042a9551908bebd25b8523d5f9769d79181"
Expand Down Expand Up @@ -2749,6 +2764,19 @@ crypto-random-string@^1.0.0:
version "1.0.0"
resolved "https://registry.yarnpkg.com/crypto-random-string/-/crypto-random-string-1.0.0.tgz#a230f64f568310e1498009940790ec99545bca7e"

css-select@~1.2.0:
version "1.2.0"
resolved "https://registry.yarnpkg.com/css-select/-/css-select-1.2.0.tgz#2b3a110539c5355f1cd8d314623e870b121ec858"
dependencies:
boolbase "~1.0.0"
css-what "2.1"
domutils "1.5.1"
nth-check "~1.0.1"

css-what@2.1:
version "2.1.3"
resolved "https://registry.yarnpkg.com/css-what/-/css-what-2.1.3.tgz#a6d7604573365fe74686c3f311c56513d88285f2"

currently-unhandled@^0.4.1:
version "0.4.1"
resolved "https://registry.yarnpkg.com/currently-unhandled/-/currently-unhandled-0.4.1.tgz#988df33feab191ef799a61369dd76c17adf957ea"
Expand Down Expand Up @@ -2950,6 +2978,37 @@ doctrine@^2.1.0:
dependencies:
esutils "^2.0.2"

dom-serializer@0, dom-serializer@~0.1.1:
version "0.1.1"
resolved "https://registry.yarnpkg.com/dom-serializer/-/dom-serializer-0.1.1.tgz#1ec4059e284babed36eec2941d4a970a189ce7c0"
dependencies:
domelementtype "^1.3.0"
entities "^1.1.1"

domelementtype@1, domelementtype@^1.3.0, domelementtype@^1.3.1:
version "1.3.1"
resolved "https://registry.yarnpkg.com/domelementtype/-/domelementtype-1.3.1.tgz#d048c44b37b0d10a7f2a3d5fee3f4333d790481f"

domhandler@^2.3.0:
version "2.4.2"
resolved "https://registry.yarnpkg.com/domhandler/-/domhandler-2.4.2.tgz#8805097e933d65e85546f726d60f5eb88b44f803"
dependencies:
domelementtype "1"

domutils@1.5.1:
version "1.5.1"
resolved "https://registry.yarnpkg.com/domutils/-/domutils-1.5.1.tgz#dcd8488a26f563d61079e48c9f7b7e32373682cf"
dependencies:
dom-serializer "0"
domelementtype "1"

domutils@^1.5.1:
version "1.7.0"
resolved "https://registry.yarnpkg.com/domutils/-/domutils-1.7.0.tgz#56ea341e834e06e6748af7a1cb25da67ea9f8c2a"
dependencies:
dom-serializer "0"
domelementtype "1"

dot-prop@^4.1.0:
version "4.2.0"
resolved "https://registry.yarnpkg.com/dot-prop/-/dot-prop-4.2.0.tgz#1f19e0c2e1aa0e32797c49799f2837ac6af69c57"
Expand Down Expand Up @@ -3458,6 +3517,10 @@ ensure-posix-path@^1.0.0, ensure-posix-path@^1.0.1, ensure-posix-path@^1.0.2:
version "1.0.2"
resolved "https://registry.yarnpkg.com/ensure-posix-path/-/ensure-posix-path-1.0.2.tgz#a65b3e42d0b71cfc585eb774f9943c8d9b91b0c2"

entities@^1.1.1:
version "1.1.2"
resolved "https://registry.yarnpkg.com/entities/-/entities-1.1.2.tgz#bdfa735299664dfafd34529ed4f8522a275fea56"

entities@~1.1.1:
version "1.1.1"
resolved "https://registry.yarnpkg.com/entities/-/entities-1.1.1.tgz#6e5c2d0a5621b5dadaecef80b90edfb5cd7772f0"
Expand Down Expand Up @@ -4645,6 +4708,17 @@ hosted-git-info@^2.1.4, hosted-git-info@^2.6.0:
version "2.7.1"
resolved "https://registry.yarnpkg.com/hosted-git-info/-/hosted-git-info-2.7.1.tgz#97f236977bd6e125408930ff6de3eec6281ec047"

htmlparser2@^3.9.1:
version "3.10.1"
resolved "https://registry.yarnpkg.com/htmlparser2/-/htmlparser2-3.10.1.tgz#bd679dc3f59897b6a34bb10749c855bb53a9392f"
dependencies:
domelementtype "^1.3.1"
domhandler "^2.3.0"
domutils "^1.5.1"
entities "^1.1.1"
inherits "^2.0.1"
readable-stream "^3.1.1"

http-cache-semantics@^3.8.1:
version "3.8.1"
resolved "https://registry.yarnpkg.com/http-cache-semantics/-/http-cache-semantics-3.8.1.tgz#39b0e16add9b605bf0a9ef3d9daaf4843b4cacd2"
Expand Down Expand Up @@ -5761,7 +5835,7 @@ lodash.values@~2.3.0:
dependencies:
lodash.keys "~2.3.0"

lodash@4.17.11, lodash@^4.17.11:
lodash@4.17.11, lodash@^4.15.0, lodash@^4.17.11:
version "4.17.11"
resolved "https://registry.yarnpkg.com/lodash/-/lodash-4.17.11.tgz#b39ea6229ef607ecd89e2c8df12536891cac9b8d"

Expand Down Expand Up @@ -6364,6 +6438,12 @@ npmlog@^4.0.0, npmlog@^4.0.2:
gauge "~2.7.3"
set-blocking "~2.0.0"

nth-check@~1.0.1:
version "1.0.2"
resolved "https://registry.yarnpkg.com/nth-check/-/nth-check-1.0.2.tgz#b2bd295c37e3dd58a3bf0700376663ba4d9cf05c"
dependencies:
boolbase "~1.0.0"

number-is-nan@^1.0.0:
version "1.0.1"
resolved "https://registry.yarnpkg.com/number-is-nan/-/number-is-nan-1.0.1.tgz#097b602b53422a522c1afb8790318336941a011d"
Expand Down Expand Up @@ -6615,7 +6695,7 @@ parse-url@^5.0.0:
parse-path "^4.0.0"
protocols "^1.4.0"

parse5@^3.0.3:
parse5@^3.0.1, parse5@^3.0.3:
version "3.0.3"
resolved "https://registry.yarnpkg.com/parse5/-/parse5-3.0.3.tgz#042f792ffdd36851551cf4e9e066b3874ab45b5c"
dependencies:
Expand Down Expand Up @@ -6965,6 +7045,14 @@ read-pkg@^1.0.0:
string_decoder "~1.1.1"
util-deprecate "~1.0.1"

readable-stream@^3.1.1:
version "3.4.0"
resolved "https://registry.yarnpkg.com/readable-stream/-/readable-stream-3.4.0.tgz#a51c26754658e0a3c21dbf59163bd45ba6f447fc"
dependencies:
inherits "^2.0.3"
string_decoder "^1.1.1"
util-deprecate "^1.0.1"

readable-stream@~1.0.2:
version "1.0.34"
resolved "https://registry.yarnpkg.com/readable-stream/-/readable-stream-1.0.34.tgz#125820e34bc842d2f2aaafafe4c2916ee32c157c"
Expand Down Expand Up @@ -7880,6 +7968,12 @@ string_decoder@0.10, string_decoder@~0.10.x:
version "0.10.31"
resolved "https://registry.yarnpkg.com/string_decoder/-/string_decoder-0.10.31.tgz#62e203bc41766c6c28c9fc84301dab1c5310fa94"

string_decoder@^1.1.1:
version "1.2.0"
resolved "https://registry.yarnpkg.com/string_decoder/-/string_decoder-1.2.0.tgz#fe86e738b19544afe70469243b2a1ee9240eae8d"
dependencies:
safe-buffer "~5.1.0"

string_decoder@~1.1.1:
version "1.1.1"
resolved "https://registry.yarnpkg.com/string_decoder/-/string_decoder-1.1.1.tgz#9cf1611ba62685d7030ae9e4ba34149c3af03fc8"
Expand Down Expand Up @@ -8394,7 +8488,7 @@ username@^1.0.1:
dependencies:
meow "^3.4.0"

util-deprecate@^1.0.2, util-deprecate@~1.0.1:
util-deprecate@^1.0.1, util-deprecate@^1.0.2, util-deprecate@~1.0.1:
version "1.0.2"
resolved "https://registry.yarnpkg.com/util-deprecate/-/util-deprecate-1.0.2.tgz#450d4dc9fa70de732762fbd2d4a28981419a0ccf"

Expand Down