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

[WIP] FastBoot compatibility #819

Closed

Conversation

Projects
None yet
4 participants
@sivakumar-kailasam
Copy link
Contributor

sivakumar-kailasam commented Jun 27, 2017

Pages:

  • Index
  • Install
  • Logout
  • keywords
  • individual keyword
  • Individual Crate
  • search
  • Login
  • User

@sivakumar-kailasam sivakumar-kailasam changed the title Index page is now FastBoot compatible [WIP] : Index page is now FastBoot compatible Jun 27, 2017

@sivakumar-kailasam sivakumar-kailasam changed the title [WIP] : Index page is now FastBoot compatible WIP | FastBoot compatibility Jun 27, 2017

@sivakumar-kailasam sivakumar-kailasam changed the title WIP | FastBoot compatibility [WIP] FastBoot compatibility Jun 27, 2017

sivakumar-kailasam and others added some commits Jun 27, 2017

this.setProperties({
'flashError': this.get('nextFlashError'),
'nextFlashError': null
});

This comment has been minimized.

@Turbo87

Turbo87 Jun 28, 2017

Member

how is that related to fastboot?

This comment has been minimized.

@sivakumar-kailasam

sivakumar-kailasam Jun 29, 2017

Author Contributor

Not related. I just grouped couple of individual set calls. Should've probably kept in a different commit.

url = `${protocol}://${host}`;
}
return url;
}),

This comment has been minimized.

@Turbo87

Turbo87 Jun 28, 2017

Member

comment here might be valuable to understand why we need this

activate() {
Ember.$.getJSON('/logout', () => {
fetch(`${this.get('appURL')}/logout`, () => {

This comment has been minimized.

@Turbo87

Turbo87 Jun 28, 2017

Member

should probably use ajax() for consistency for now

},

fastboot: {
hostWhitelist: ['crates.io', /^localhost:\d+$/]

This comment has been minimized.

@Turbo87

Turbo87 Jun 28, 2017

Member

can we add localhost only in dev mode?

* during Server Side Rendering(SSR) via FastBoot,
* the JS Fetch API doesn't work with absolute urls.
* This property gives the URL prefix for those network calls.
* */

This comment has been minimized.

@Turbo87

Turbo87 Jun 29, 2017

Member

isn't it the other way around? it doesn't work with relative URLs, right?

This comment has been minimized.

@sivakumar-kailasam

sivakumar-kailasam Jun 29, 2017

Author Contributor

danngit, you're right. I don't think we need it anymore if I use ajax service.

@@ -1,20 +1,20 @@
import Ember from 'ember';
import ajax from 'ic-ajax';

const { inject: { service } } = Ember;

This comment has been minimized.

@Turbo87

Turbo87 Jun 29, 2017

Member

this will make it harder for the new modules codemod to transform it into an import from @ember/service... I'd rather just keep using the globals for now.

This comment has been minimized.

@sivakumar-kailasam

sivakumar-kailasam Jun 29, 2017

Author Contributor

Oh so that's why you've used full form everywhere. Got it!

This comment has been minimized.

@Turbo87

Turbo87 Jun 29, 2017

Member

yep, and I don't multi-level destructuring that much 😉

@sivakumar-kailasam sivakumar-kailasam force-pushed the sivakumar-kailasam:das-fastboot branch from e158061 to ffbfcbe Jun 29, 2017

sivakumar-kailasam added some commits Jun 29, 2017

@Turbo87 Turbo87 referenced this pull request Sep 13, 2017

Open

Use fastboot #204

@sivakumar-kailasam

This comment has been minimized.

Copy link
Contributor Author

sivakumar-kailasam commented Sep 25, 2017

Closing this PR in favor of smaller PRs to get this repo to be fastboot compatible and then land one PR with the fastboot specific changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.