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

Embroider build replaces Fastboot generated package.json #160

Closed
dnalagatla opened this issue May 7, 2019 · 10 comments
Closed

Embroider build replaces Fastboot generated package.json #160

dnalagatla opened this issue May 7, 2019 · 10 comments

Comments

@dnalagatla
Copy link
Contributor

My PR - ember-fastboot/ember-cli-fastboot#690 moved the implementation form the postProcessTree to treeForPublic hook. But after doing that, I noticed embroider build doesn't add the package.json generated by ember-cli-fastboot addon. Instead, it keeps the package.json created by enbroider build in the dist folder. I am thinking some diff code is assuming the package.json generated as part of Fastboot tree is similar to package.json generated by embroider build and keeps embroider build related package.json

To Repro the issue

  1. clone - https://github.com/dnalagatla/my-embroider-test.git
  2. run yarn install
  3. ember s
  4. launch http://localhost:4200

page will show following error:

Error: /var/folders/tv/fgbbtzvd1f1dbv45y7r8l09m000ty_/T/broccoli-82821i0tVejhn4W1b/out-176-packager_runner_embroider_webpack/package.json was malformed or did not contain a manifest. Ensure that you have a compatible version of ember-cli-fastboot.
    at EmberApp.readPackageJSON (/Users/dnalagat/DEVELOPMENT/ember-cli-fastboot/node_modules/fastboot/src/ember-app.js:362:13)
    at new EmberApp (/Users/dnalagat/DEVELOPMENT/ember-cli-fastboot/node_modules/fastboot/src/ember-app.js:34:23)
    at FastBoot._buildEmberApp (/Users/dnalagat/DEVELOPMENT/ember-cli-fastboot/node_modules/fastboot/src/index.js:114:17)
    at new FastBoot (/Users/dnalagat/DEVELOPMENT/ember-cli-fastboot/node_modules/fastboot/src/index.js:52:10)
    at app.use (/Users/dnalagat/DEVELOPMENT/ember-cli-fastboot/index.js:318:29)
    at Layer.handle [as handle_request] (/Users/dnalagat/DEVELOPMENT/test-pemberly-bpr/test-ember/my-embroider-test/node_modules/express/lib/router/layer.js:95:5)
    at trim_prefix (/Users/dnalagat/DEVELOPMENT/test-pemberly-bpr/test-ember/my-embroider-test/node_modules/express/lib/router/index.js:317:13)
    at /Users/dnalagat/DEVELOPMENT/test-pemberly-bpr/test-ember/my-embroider-test/node_modules/express/lib/router/index.js:284:7
    at Function.process_params (/Users/dnalagat/DEVELOPMENT/test-pemberly-bpr/test-ember/my-embroider-test/node_modules/express/lib/router/index.js:335:12)
    at next (/Users/dnalagat/DEVELOPMENT/test-pemberly-bpr/test-ember/my-embroider-test/node_modules/express/lib/router/index.js:275:10) 

This error is thrown because manifest property is missing in generated package.json.

I am thinking the fastboot generation of package.json should move to postBuild hook instead of using treeForPublic. what do you guys think @ef4 @stefanpenner @rwjblue

Related to issue - #112
I missed this error during testing of my PR as the git repo mentioned in the above issue didn't include fastboot and I tested assuming ember-cli-fastboot was included as dependency. :(

@ef4
Copy link
Contributor

ef4 commented May 8, 2019

Moving to postBuild won't make compatibility with current embroider any easier. AFAIK we don't even run that hook.

I think we should deal with this in embroider. Probably by detecting the package.json in the public tree and merging it into our own.

@dnalagatla
Copy link
Contributor Author

Hi @ef4,
@rwjblue and myself investigated where embroider build is overwriting package.json generated by ember-cli-fastboot. I created this PR to merge package.json properties into package.json created by embroider.

@dnalagatla
Copy link
Contributor Author

dnalagatla commented May 17, 2019

After adding the above fix in the test app I noticed following issues with Embroider+Fastboot build:

  • Missing the <app>.js file used by fastboot. - I think we may have to evaluate index.html to load all the files (vendor and app chunk JS) in the fastboot sandbox.
  • With fastboot query param set to false doesn't load application correctly. App loads fine without fastboot as a dependency.

Investigating ^ issues. Will update here.
cc: @stefanpenner @rwjblue @ef4

@ef4
Copy link
Contributor

ef4 commented May 18, 2019

This was closed by #178.

@ef4
Copy link
Contributor

ef4 commented Nov 8, 2019

Reopened because the fix was reverted.

simonihmig added a commit to ember-bootstrap/ember-bootstrap that referenced this issue Jan 7, 2020
@jelhan
Copy link
Contributor

jelhan commented Feb 20, 2020

Should be fixed by #383 if I didn't missed something.

@simonihmig
Copy link
Collaborator

Although the PR mentioned above seems to address the issue, I am still running into it, with the latest embroider release (0.27.0).

You can see that quite well when building the fastboot-app test-package in this repo: dist/package.json contains none of the FastBoot manifest stuff that you see in a classic build!

In an embroider build (ember build):

package.json
{
  "name": "fastboot-app",
  "version": "0.0.0",
  "private": true,
  "description": "Small description for fastboot-app 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:*",
    "lint:hbs": "ember-template-lint .",
    "lint:js": "eslint .",
    "start": "ember serve",
    "test": "npm-run-all lint:* test:*",
    "test:ember": "ember test --test-port=0",
    "test:ember-production": "ember test --test-port=0 --environment=production",
    "test:fastboot": "qunit fastboot-tests",
    "test:fastboot-production": "FASTBOOT_APP_PROD=true qunit fastboot-tests"
  },
  "devDependencies": {
    "@ember/optional-features": "^1.3.0",
    "@embroider/compat": "0.27.0",
    "@embroider/core": "0.27.0",
    "@embroider/sample-lib": "0.0.0",
    "@embroider/webpack": "0.27.0",
    "@glimmer/component": "^1.0.0",
    "@glimmer/tracking": "^1.0.0",
    "babel-eslint": "^10.1.0",
    "broccoli-asset-rev": "^3.0.0",
    "ember-auto-import": "^1.5.3",
    "ember-cli": "~3.18.0",
    "ember-cli-app-version": "^3.2.0",
    "ember-cli-babel": "^7.20.5",
    "ember-cli-dependency-checker": "^3.2.0",
    "ember-cli-fastboot": "^2.2.2",
    "ember-cli-fastboot-testing": "^0.4.0",
    "ember-cli-htmlbars": "^4.3.1",
    "ember-cli-inject-live-reload": "^2.0.2",
    "ember-cli-sri": "^2.1.1",
    "ember-cli-uglify": "^3.0.0",
    "ember-export-application-global": "^2.0.1",
    "ember-fetch": "^8.0.1",
    "ember-load-initializers": "^2.1.1",
    "ember-maybe-import-regenerator": "^0.1.6",
    "ember-qunit": "^4.6.0",
    "ember-resolver": "^8.0.0",
    "ember-source": "~3.18.0",
    "ember-template-lint": "^2.6.0",
    "ember-welcome-page": "^4.0.0",
    "eslint": "^6.8.0",
    "eslint-plugin-ember": "^8.4.0",
    "eslint-plugin-node": "^9.0.0",
    "fastboot": "^3.1.0",
    "fastboot-addon": "0.0.0",
    "jsdom": "^16.2.2",
    "loader.js": "^4.7.0",
    "npm-run-all": "^4.1.5",
    "qunit": "^2.10.0",
    "qunit-dom": "^1.2.0"
  },
  "engines": {
    "node": "10.* || >= 12"
  },
  "ember": {
    "edition": "octane"
  },
  "volta": {
    "extends": "../../package.json"
  },
  "keywords": [
    "ember-addon"
  ],
  "ember-addon": {
    "type": "app",
    "version": 2,
    "assets": [
      "/assets/fastboot-app.css",
      "ember-cli-fastboot/app-factory.js",
      "ember-fetch/fetch-fastboot.js",
      "ember-welcome-page/images/construction.png",
      "fastboot-addon/sample.js",
      "assets/embroider_macros_fastboot_init.js",
      "robots.txt",
      "testem.js",
      "index.html",
      "tests/index.html",
      "package.json",
      "assets/vendor.map",
      "assets/vendor.css.map",
      "assets/test-support.map",
      "assets/test-support.css.map"
    ],
    "template-compiler": {
      "filename": "_template_compiler_.js",
      "isParallelSafe": true
    },
    "babel": {
      "filename": "_babel_config_.js",
      "isParallelSafe": true,
      "majorVersion": 7,
      "fileFilter": "_babel_filter_.js"
    },
    "resolvable-extensions": [
      ".wasm",
      ".mjs",
      ".js",
      ".json",
      ".hbs"
    ],
    "root-url": "/",
    "auto-upgraded": true
  },
  "dependencies": {
    "abortcontroller-polyfill": "^1.4.0",
    "node-fetch": "^2.6.0"
  },
  "fastboot": {
    "schemaVersion": 5,
    "htmlEntrypoint": "index.html",
    "moduleWhitelist": [
      "node-fetch",
      "abortcontroller-polyfill",
      "abortcontroller-polyfill/dist/cjs-ponyfill"
    ]
  }
}

In a classic build (CLASSIC=true ember build):

package.json
{
  "dependencies": {
    "abortcontroller-polyfill": "^1.4.0",
    "node-fetch": "^2.6.0"
  },
  "fastboot": {
    "appName": "fastboot-app",
    "config": {
      "fastboot-app": {
        "APP": {
          "autoboot": false,
          "name": "fastboot-app",
          "version": "0.0.0+296dd523"
        },
        "EmberENV": {
          "EXTEND_PROTOTYPES": {
            "Date": false
          },
          "FEATURES": {},
          "_APPLICATION_TEMPLATE_WRAPPER": false,
          "_DEFAULT_ASYNC_OBSERVERS": true,
          "_JQUERY_INTEGRATION": false,
          "_TEMPLATE_ONLY_GLIMMER_COMPONENTS": true
        },
        "environment": "development",
        "exportApplicationGlobal": true,
        "fastboot": {
          "hostWhitelist": [
            "/localhost:\\d+$/"
          ]
        },
        "locationType": "auto",
        "modulePrefix": "fastboot-app",
        "rootURL": "/"
      }
    },
    "hostWhitelist": [
      "/localhost:\\d+$/"
    ],
    "manifest": {
      "appFiles": [
        "assets/fastboot-app.js",
        "assets/fastboot-app-fastboot.js"
      ],
      "htmlFile": "index.html",
      "vendorFiles": [
        "assets/vendor.js",
        "ember-fetch/fetch-fastboot.js",
        "fastboot-addon/sample.js"
      ]
    },
    "moduleWhitelist": [
      "node-fetch",
      "abortcontroller-polyfill",
      "abortcontroller-polyfill/dist/cjs-ponyfill"
    ],
    "schemaVersion": 3
  }
}

The error that I see is not the one mentioned in the original comment, but rather You must provide a hostWhitelist to retrieve the host when trying to read request.host. But this is also caused by the package.json being not what Fastboot expects, specifically the missing hostWhitelist field.

Will provide a PR with a failing test...

simonihmig added a commit to simonihmig/embroider that referenced this issue Oct 20, 2020
Adds accessing `request.host` to `fastboot-app` that reproduces the missing FastBoot package.json data (here `hostWhitelist`) reported in embroider-build#160.
@simonihmig
Copy link
Collaborator

Here is a failing test: #574

@ef4
Copy link
Contributor

ef4 commented Oct 20, 2020

I don't think your problem is related to this issue. In your package.json, you can see that we did emit fastboot info, it's just that it's using schemaVersion: 5 which is much smaller (because it relies on the contents of index.html instead of duplicating that info inside package.json).

@simonihmig
Copy link
Collaborator

You are right, I didn't pay enough attention to the FastBoot manifest data it seems. It was just obvious that it was totally different, so I wrongly assumed Embroider had omitted something. Until I realized that Embroider has some custom adapter for ember-cli-fastboot, which seems to explain the vast difference between builds.

I think I have found a fix, will update my PR, then let's move the discussion over there...

So, as my issue is indeed not really related to this issue, which seems to have been fixed by now, I think we can close this...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants