Skip to content
This repository has been archived by the owner on Dec 20, 2018. It is now read-only.

Fastboot fails #5

Closed
ewoutp opened this issue Nov 8, 2016 · 17 comments
Closed

Fastboot fails #5

ewoutp opened this issue Nov 8, 2016 · 17 comments
Labels

Comments

@ewoutp
Copy link

ewoutp commented Nov 8, 2016

I've tried this module in combination with Ember fastboot, but I get errors in fastboot (that I do not get without this module).

I've only installed this module (ember install..) and not done anything else.

@san650 san650 added the bug label Nov 8, 2016
@san650
Copy link
Owner

san650 commented Nov 8, 2016

Thanks for reporting the issue @ewoutp. The addon is still a work in progress but the early feedback is welcome.

I'll try to tackle fastboot support asap.

@ewoutp
Copy link
Author

ewoutp commented Nov 8, 2016

Great!.

I was looking for a plugin like this, so I'm happy to test stuff.

@san650
Copy link
Owner

san650 commented Dec 11, 2016

@ewoutp I was trying to reproduce this error but it seems that everything works fine with the latest version.

Can you try to reproduce the error? Can you also provide more info about the error?

Thanks!

@san650
Copy link
Owner

san650 commented Dec 14, 2016

I couldn't reproduce the issue so I'm closing it for now.

@ewoutp please, feel free to open it again if you can reproduce the issue.

@san650 san650 closed this as completed Dec 14, 2016
@ewoutp
Copy link
Author

ewoutp commented Dec 15, 2016

I'll test it as soon as I have a little time for it.
Thx!

@teddyzeenny
Copy link
Contributor

Hi @san650. I also couldn't use this addon with fastboot. I was able to track down the problem:

  • This addon adds json to the replaceExtensions array in broccoli-asset-rev's fingerprint options (to fingerprint the assets in manifest.json).
  • Fastboot creates a file called fastbootAssetMap.json in dist to map the original asset URLs to the newly fingerprinted ones.
  • Since we are now fingerprinting assets in json files (because of step 1) fastbootAssetMap.json's asset URLs are being fingerprinted as well (which makes the asset map useless since it's now mapping fingerprinted assets to fingerprinted assets).

In order to reproduce this you will need to be using fastboot with the production build (so that fingerprinting is enabled).

@teddyzeenny
Copy link
Contributor

I just found a related issue ember-cli/broccoli-asset-rev#91.

@san650
Copy link
Owner

san650 commented Dec 15, 2016

Thanks @ewoutp and @teddyzeenny ! I'll open the issue again.

@san650 san650 reopened this Dec 15, 2016
@teddyzeenny
Copy link
Contributor

I think adding json to replaceExtensions globally is going to have unexpected side effects not limited to just fastboot.

@san650
Copy link
Owner

san650 commented Dec 15, 2016

I was playing with the ignore option of broccoli-asset-rev, the option is forwarded to broccoli-asset-write and allows you to ignore the given paths. It kind of works but it solves only the fastbootAssetMap.json case.

You're right about that adding json to replaceExtensions could break other addons/apps.

Another approach I'm trying right now is to rename the manifest.json file to manifest.x.json and adding x.json to replaceExtensions. The idea is to rename the manifest.x.json file to manifest.json after broccoli-asset-rev runs. It's just an experiment but it might be a plausible solution (maybe we can invent a different extension name but x.json works for now).

@teddyzeenny What do you think?

@san650
Copy link
Owner

san650 commented Dec 15, 2016

@teddyzeenny I've opened an experimental PR with the ignore solution. See #19

Do you want to work on this issue? I was just doing a quick experiment.

@teddyzeenny
Copy link
Contributor

@san650 .x.json approach is the workaround I did in my app :) Used: manifest.ember-web-app.json and renamed it back to manifest.json in postBuild hook.

@san650
Copy link
Owner

san650 commented Dec 15, 2016

@teddyzeenny Do you want to open a PR with this solution? I like the ember-web-app.json name for the extension.

If not, I'll try to implement it later this week.

Thanks for helping triaging this bug!

@teddyzeenny
Copy link
Contributor

teddyzeenny commented Dec 15, 2016

@san650 sure I can open a PR. Would you like to keep it as manifest.ember-web-app.json and update the <link rel="manifest"> href, or rename it to manifest.json in postBuild?

@san650
Copy link
Owner

san650 commented Dec 15, 2016

I prefer to rename it to manifest.json in postBuild. I would like to make the name configurable in the future so we'll need the postBuild step either way.

@san650
Copy link
Owner

san650 commented Dec 15, 2016

Thanks @teddyzeenny for fixing the bug! 👏

@san650
Copy link
Owner

san650 commented Dec 15, 2016

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

No branches or pull requests

3 participants