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

Update for fastboot 1.0 #29

Merged
merged 8 commits into from May 27, 2017

Conversation

scottmessinger
Copy link
Contributor

This isn't working yet :-( But I think the basics are there. Anyone know why I'm seeing this error message?

Error: ENOENT: no such file or directory,
open '/app/tmp/broccoli_merge_trees-output_path-3ahQH1aD.tmp/app/node_modules/ember-fetch/assets/fastboot-fetch.js'

@scottmessinger
Copy link
Contributor Author

@kratiahuja
Copy link
Collaborator

Hey Scott! I'll take a look at that error by end of day and let you know.

index.js Outdated

//add node version of fetch.js into fastboot package.json manifest vendorFiles array
updateFastBootManifest: function(manifest) {
var fastbootFetchPath = path.resolve(__dirname + '/assets/fastboot-fetch.js');
Copy link
Collaborator

Choose a reason for hiding this comment

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

The reason you are seeing the error is because of this line. This will give you an absolute path. The fastboot manifest takes the relative path to the dist or tmp directory from it is going to serve the assets. The reason being it serves everything off a relative path that is given to it on boot.

Therefore, this asset needs to be part of the app build assets (ie in dist).

My recommendation would be:

  1. Move the contents of assets/fastboot-fetch.js to public/fastboot-fetch.js and delete assets/fastboot-fetch.js
  2. Once it is in public folder, ember-cli will automatically build it and put it in dist/ember-fetch/fastboot-fetch.js after the build.
  3. You can update this function to just be:
updateFastBootManifest: function(manifest) {
  manifest.vendorFiles.push('ember-fetch/fastboot-fetch.js');
  return manifest;
}

See the similar changes here.

Also see the migration guide here

@scottmessinger
Copy link
Contributor Author

@kratiahuja Thanks! That fixed it!!!

@scottmessinger scottmessinger changed the title WIP Update for fastboot 1.0 Update for fastboot 1.0 May 18, 2017
@kratiahuja
Copy link
Collaborator

LGTM. @scottmessinger thanks for tackling this. Can you confirm you tested all three scenarios listed here: https://gist.github.com/kratiahuja/d22de0fb1660cf0ef58f07a6bcbf1a1c#testing-guidelines ? Once you confirm, this should be good to be merged. Ideally we should add fastboot tests, but I'll open an issue in this repo.

@kratiahuja
Copy link
Collaborator

@scottmessinger pinging again if you have cycles to test this? If not, I'll test this over the weekend and merge.

@scottmessinger
Copy link
Contributor Author

@kratiahuja I'm sorry! I haven't gotten a chance to test the three scenarios and I'm not sure when I'll get a chance to do it thoroughly :-(

@kratiahuja
Copy link
Collaborator

No worries, I'll test it over the weekend.

@kratiahuja
Copy link
Collaborator

Confirmed this works with the beta and rc builds. Merging this...

@kratiahuja
Copy link
Collaborator

Thanks @scottmessinger !

@kratiahuja kratiahuja merged commit 994c0df into ember-cli:master May 27, 2017
@kratiahuja
Copy link
Collaborator

@stefanpenner I don't think I am an npm owner. Can we please do a 2.0.1 release?

@kratiahuja
Copy link
Collaborator

@stefanpenner can we release 2.0.1?

@stefanpenner
Copy link
Collaborator

@kratiahuja you are now an owner.

@stefanpenner
Copy link
Collaborator

released as v2.1.0 🎉

@cibernox
Copy link
Collaborator

cibernox commented Jun 6, 2017

However I'm getting Cannot read property 'then' of undefined when I use fetch in fastboot mode. Has anyone see the same thing?

@scottmessinger
Copy link
Contributor Author

@cibernox I haven't seen that -- it makes me wonder if the fetch library that ember-fetch depends on is loading.

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.

None yet

4 participants