-
-
Notifications
You must be signed in to change notification settings - Fork 80
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
Fix fetch public/fastboot-fetch.js module definition for Fastboot #167
Conversation
What if ember-fetch is never the top level addon? What if it is only used as a subdep with several versions? |
@Turbo87 So if |
9c5ab46
to
978f2d8
Compare
@xg-wang this PR seems to contain a lot of unrelated changes now. can you make sure to open dedicated PRs for those unrelated changes? |
40f4ddb
to
e970738
Compare
#171 is merged, this good to go? |
Define fetch module with a setupFastboot method export to set host and protocol for every instance. Rename public/fastboot-fetch.js to public/fetch-fastboot.js to avoid lower version ember-fetch overwrite this file. Overrides treeForPublic in index.js to only include public asset if top level addon to avoid any future rename.
We may want to rethink this design a bit now that |
Agree |
+1 |
Fix #166, depends on #171
In v6.2.0
fetch/setup
was added topublic/fastboot-fetch.js
so we can setupfetch
module with fastboot request info. However this file will be overwritten by any addon's dependencyember-fetch
based on last-write-win. To fix this:lower version ember-fetch overwrite this file.
level addon.
For example, top level addon
ember-fetch
is 6.2.1, transitive dependency is anything before 6.2.0 will result in this dist:6.2.1 as top level addon will
manifest.vendorFiles.push('ember-fetch/fetch-fastboot.js');
so thefastboot.js
will just be ignored.This commit also defines fetch module with a
setupFastboot
method to set host andprotocol for every instance. This could avoid module redefinition.