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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Load config based on storeConfigInMeta option #228

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

@kamalaknn
Copy link

@kamalaknn kamalaknn commented Oct 18, 2016

I've tried to feature pair storeConfigInMeta option from ember-cli. Ported from ember-cli's implementation.

I am unsure about this.app for nested engines though. Feedback welcome 馃槃

@nathanhammond
Copy link
Contributor

@nathanhammond nathanhammond commented Oct 26, 2016

@trentmwillis and I talked about something like this today for one of our own use cases. There is a weird interplay of issues here between Fastboot, lazy engines, app code, and our current asset manifest insertion strategy (which consistently rewrites the meta tag as time goes on since we never know when we're "done").

This is something that needs further design discussion prior to us deciding if and how to land a feature that does this. Regardless, this is a fantastic straw man.

Would you be willing to rebase onto master and get tests passing?

@kamalaknn
Copy link
Author

@kamalaknn kamalaknn commented Oct 26, 2016

@nathanhammond Sure. I will give it a go 馃憤

@kamalaknn kamalaknn force-pushed the kamalaknn:respect-store-config-in-meta branch from 4aa572d to 9eed103 Oct 26, 2016
@kamalaknn kamalaknn changed the base branch from v0.3 to master Oct 26, 2016
@kamalaknn kamalaknn force-pushed the kamalaknn:respect-store-config-in-meta branch from 9eed103 to e5b96b8 Oct 26, 2016
@kamalaknn
Copy link
Author

@kamalaknn kamalaknn commented Oct 26, 2016

Rebased and moved against master. Travis is green now 馃槂

@joebartels
Copy link

@joebartels joebartels commented Mar 22, 2017

Could someone share status on this story. I still have this issue. Is there a known workaround?

Using
Ember-Engines v0.5.0
Ember v2.12.0
Ember-Cli-Fastboot 1.0.0-beta.15

Copy link
Member

@rwjblue rwjblue left a comment

@kamalaknn - Sorry for letting this linger so long. I am reading through the diff, and it seems like there is a missing file (engine-config-from-meta.js is not included in this PR). Can you take a look?

@krisselden
Copy link
Collaborator

@krisselden krisselden commented Aug 8, 2017

We don't want to inline the config into the JS. ember-fastboot needs a way an addon can add to the sandbox so that engines can get the config from there.

Also, it should be ok for an engine not to have any configuration.

@kamalaknn
Copy link
Author

@kamalaknn kamalaknn commented Aug 9, 2017

@rwjblue I could see that engine-config-from-meta.js is already on the project. I could be wrong as well, it's been quite some time since I worked with ember/ember-cli. I might have lost context.

@rwjblue rwjblue force-pushed the kamalaknn:respect-store-config-in-meta branch from e5b96b8 to 125f3d2 Aug 9, 2017
@skaterdav85
Copy link

@skaterdav85 skaterdav85 commented Oct 10, 2017

Does anyone have a workaround for storeConfigInMeta: false?

@ASH-Michael
Copy link

@ASH-Michael ASH-Michael commented Oct 30, 2018

@rwjblue It looks like this PR is just about at the finish line after the conflicts are resolved. Is there anything else that this PR still needs?

@chendraayan
Copy link

@chendraayan chendraayan commented Feb 18, 2019

Just wondering what happened to this story. Does anyone have a workaround for storeConfigInMeta: false?

@villander
Copy link
Member

@villander villander commented Mar 27, 2019

@rwjblue do we still need this? what is the relation of storeConfigInMeta with ember engines and fastboot?

@puopg
Copy link

@puopg puopg commented Mar 27, 2019

Id love to be able to read the config in without the meta tags

@villander
Copy link
Member

@villander villander commented Mar 27, 2019

@stefanpenner I think that we need of a enhancement to read the config without meta tag, what about?

@puopg
Copy link

@puopg puopg commented Mar 27, 2019

Given how old this PR is, and since I cant update this one, i opened #633

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

10 participants