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

Preparation for Fastboot (1/3) #1886

Merged
merged 2 commits into from
Nov 11, 2019
Merged

Preparation for Fastboot (1/3) #1886

merged 2 commits into from
Nov 11, 2019

Conversation

kzys
Copy link
Contributor

@kzys kzys commented Nov 7, 2019

This PR contains Fastboot-related fixes (from #1832) except for actually enabling Fastboot.

After merging this change, I'm going to submit a PR that updates package.json to include Fastboot in the dependencies.

Then I will submit a PR that enables Fastboot, based on USE_FASTBOOT env variable.

@rust-highfive
Copy link

r? @smarnach

(rust_highfive has picked a reviewer for you, use r? to override)

@carols10cents
Copy link
Member

Hm, when I try to run this locally with npm run start:local (and the backend running locally with cargo run --bin server), I get this error in the browser when I visit http://localhost:4200/:

Error: You must provide a hostWhitelist to retrieve the host
    at FastBootRequest.host (/Users/carolnichols/rust/crates.io/node_modules/fastboot/src/fastboot-request.js:22:13)
    at Class._host (/var/folders/qc/0ry2bjtd7wd9q8r1c7wylplr0000gn/T/broccoli-56297ix2RRAJ9GaI0/out-416-broccoli_merge_trees/assets/addon-tree-output/ember-cli-fastboot/services/fastboot.js:25:1)
    at Class.<anonymous> (/var/folders/qc/0ry2bjtd7wd9q8r1c7wylplr0000gn/T/broccoli-56297ix2RRAJ9GaI0/out-416-broccoli_merge_trees/assets/addon-tree-output/ember-cli-fastboot/services/fastboot.js:30:1)
    at ComputedPropertyPrototype.get (/var/folders/qc/0ry2bjtd7wd9q8r1c7wylplr0000gn/T/broccoli-56297ix2RRAJ9GaI0/out-416-broccoli_merge_trees/assets/ember-metal.js:4069:1)
    at get (/var/folders/qc/0ry2bjtd7wd9q8r1c7wylplr0000gn/T/broccoli-56297ix2RRAJ9GaI0/out-416-broccoli_merge_trees/assets/ember-metal.js:3439:1)
    at Class.get (/var/folders/qc/0ry2bjtd7wd9q8r1c7wylplr0000gn/T/broccoli-56297ix2RRAJ9GaI0/out-416-broccoli_merge_trees/assets/ember-runtime/mixins/observable.js:38:1)
    at Object.patchFetchForRelativeURLs [as initialize] (/var/folders/qc/0ry2bjtd7wd9q8r1c7wylplr0000gn/T/broccoli-56297ix2RRAJ9GaI0/out-416-broccoli_merge_trees/assets/cargo/instance-initializers/setup-fetch.js:19:1)
    at /var/folders/qc/0ry2bjtd7wd9q8r1c7wylplr0000gn/T/broccoli-56297ix2RRAJ9GaI0/out-416-broccoli_merge_trees/assets/ember-application/system/engine.js:84:1
    at Vertices.each (/var/folders/qc/0ry2bjtd7wd9q8r1c7wylplr0000gn/T/broccoli-56297ix2RRAJ9GaI0/out-416-broccoli_merge_trees/assets/dag-map.js:188:1)
    at Vertices.walk (/var/folders/qc/0ry2bjtd7wd9q8r1c7wylplr0000gn/T/broccoli-56297ix2RRAJ9GaI0/out-416-broccoli_merge_trees/assets/dag-map.js:117:1)
    at DAG.each (/var/folders/qc/0ry2bjtd7wd9q8r1c7wylplr0000gn/T/broccoli-56297ix2RRAJ9GaI0/out-416-broccoli_merge_trees/assets/dag-map.js:62:1)
    at DAG.topsort (/var/folders/qc/0ry2bjtd7wd9q8r1c7wylplr0000gn/T/broccoli-56297ix2RRAJ9GaI0/out-416-broccoli_merge_trees/assets/dag-map.js:68:1)
    at Class._runInitializer (/var/folders/qc/0ry2bjtd7wd9q8r1c7wylplr0000gn/T/broccoli-56297ix2RRAJ9GaI0/out-416-broccoli_merge_trees/assets/ember-application/system/engine.js:98:1)
    at Class.runInstanceInitializers (/var/folders/qc/0ry2bjtd7wd9q8r1c7wylplr0000gn/T/broccoli-56297ix2RRAJ9GaI0/out-416-broccoli_merge_trees/assets/ember-application/system/engine.js:81:1)
    at Class._bootSync (/var/folders/qc/0ry2bjtd7wd9q8r1c7wylplr0000gn/T/broccoli-56297ix2RRAJ9GaI0/out-416-broccoli_merge_trees/assets/ember-application/system/application-instance.js:95:1)
    at /var/folders/qc/0ry2bjtd7wd9q8r1c7wylplr0000gn/T/broccoli-56297ix2RRAJ9GaI0/out-416-broccoli_merge_trees/assets/ember-application/system/engine-instance.js:57:1
    at initializePromise (/var/folders/qc/0ry2bjtd7wd9q8r1c7wylplr0000gn/T/broccoli-56297ix2RRAJ9GaI0/out-416-broccoli_merge_trees/assets/rsvp.js:397:1)
    at new Promise (/var/folders/qc/0ry2bjtd7wd9q8r1c7wylplr0000gn/T/broccoli-56297ix2RRAJ9GaI0/out-416-broccoli_merge_trees/assets/rsvp.js:877:1)
    at Class.boot (/var/folders/qc/0ry2bjtd7wd9q8r1c7wylplr0000gn/T/broccoli-56297ix2RRAJ9GaI0/out-416-broccoli_merge_trees/assets/ember-application/system/engine-instance.js:56:1)
    at buildAppInstance.then.appInstance (/Users/carolnichols/rust/crates.io/node_modules/fastboot/src/ember-app.js:289:25)
    at tryCatcher (/var/folders/qc/0ry2bjtd7wd9q8r1c7wylplr0000gn/T/broccoli-56297ix2RRAJ9GaI0/out-416-broccoli_merge_trees/assets/rsvp.js:200:1)
    at invokeCallback (/var/folders/qc/0ry2bjtd7wd9q8r1c7wylplr0000gn/T/broccoli-56297ix2RRAJ9GaI0/out-416-broccoli_merge_trees/assets/rsvp.js:372:1)
    at publish (/var/folders/qc/0ry2bjtd7wd9q8r1c7wylplr0000gn/T/broccoli-56297ix2RRAJ9GaI0/out-416-broccoli_merge_trees/assets/rsvp.js:358:1)
    at /var/folders/qc/0ry2bjtd7wd9q8r1c7wylplr0000gn/T/broccoli-56297ix2RRAJ9GaI0/out-416-broccoli_merge_trees/assets/ember-testing/ext/rsvp.js:14:1
    at invokeWithOnError (/var/folders/qc/0ry2bjtd7wd9q8r1c7wylplr0000gn/T/broccoli-56297ix2RRAJ9GaI0/out-416-broccoli_merge_trees/assets/backburner.js:214:1)
    at Queue.flush (/var/folders/qc/0ry2bjtd7wd9q8r1c7wylplr0000gn/T/broccoli-56297ix2RRAJ9GaI0/out-416-broccoli_merge_trees/assets/backburner.js:125:1)
    at DeferredActionQueues.flush (/var/folders/qc/0ry2bjtd7wd9q8r1c7wylplr0000gn/T/broccoli-56297ix2RRAJ9GaI0/out-416-broccoli_merge_trees/assets/backburner.js:278:1)
    at Backburner.end (/var/folders/qc/0ry2bjtd7wd9q8r1c7wylplr0000gn/T/broccoli-56297ix2RRAJ9GaI0/out-416-broccoli_merge_trees/assets/backburner.js:410:1)
    at Timeout.Backburner._boundAutorunEnd [as _onTimeout] (/var/folders/qc/0ry2bjtd7wd9q8r1c7wylplr0000gn/T/broccoli-56297ix2RRAJ9GaI0/out-416-broccoli_merge_trees/assets/backburner.js:372:1)
    at ontimeout (timers.js:436:11)
    at tryOnTimeout (timers.js:300:5)
    at listOnTimeout (timers.js:263:5)
    at Timer.processTimers (timers.js:223:10)

I get the same error when I run with npm run start:live (to proxy to crates.io) and npm run start (to run with fixtures).

Am I supposed to be using a different command to run locally? Do we need to set a hostWhitelist, and if so, where and to what?

Good news is that npm test works locally!

@bors
Copy link
Contributor

bors commented Nov 8, 2019

☔ The latest upstream changes (presumably #1879) made this pull request unmergeable. Please resolve the merge conflicts.

@kzys
Copy link
Contributor Author

kzys commented Nov 8, 2019

Oh, I was assuming just adding dependencies won't change the behavior and didn't check these npm commands. My bad...

In that case, let me first revert package.json changes. That at least merging this PR easier.

@kzys kzys force-pushed the fastboot-prepare branch from 47e4077 to d092fd9 Compare November 8, 2019 16:32
kzys added 2 commits November 8, 2019 08:52
- Don't use .ember-application which is added after FastBoot
  This class is added dynamically, after FastBoot rendering.

- On Acceptance Tests, the application is rendered under #ember-testing,
  not the root <body> element.
@kzys kzys force-pushed the fastboot-prepare branch from d092fd9 to 034fd45 Compare November 8, 2019 16:52
@kzys kzys changed the title Preparation for Fastboot Preparation for Fastboot (1/3) Nov 8, 2019
@carols10cents
Copy link
Member

This works now, thank you!!!

@bors r+

@bors
Copy link
Contributor

bors commented Nov 11, 2019

📌 Commit 034fd45 has been approved by carols10cents

@bors
Copy link
Contributor

bors commented Nov 11, 2019

⌛ Testing commit 034fd45 with merge 23f187a...

bors added a commit that referenced this pull request Nov 11, 2019
Preparation for Fastboot (1/3)

This PR contains Fastboot-related fixes (from #1832) except for actually enabling Fastboot.

After merging this change, I'm going to submit a PR that updates package.json to include Fastboot in the dependencies.

Then I will submit a PR that enables Fastboot, based on USE_FASTBOOT env variable.
@bors
Copy link
Contributor

bors commented Nov 11, 2019

☀️ Test successful - checks-travis
Approved by: carols10cents
Pushing 23f187a to master...

@bors bors merged commit 034fd45 into rust-lang:master Nov 11, 2019
@kzys kzys mentioned this pull request Nov 12, 2019
19 tasks
@kzys kzys deleted the fastboot-prepare branch November 12, 2019 06:38
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

Successfully merging this pull request may close these issues.

5 participants