Wiring sub-contexts doesn't work inside a nested scope #123

Closed
AnAppAMonth opened this Issue Jul 16, 2013 · 10 comments

Comments

Projects
None yet
2 participants
@AnAppAMonth

Nested scopes are created by calling new Scope(this), the second parameter options isn't provided. This causes the createContext() method to not be available in the nested scope, and therefore using the wireFactory or calling pluginApi.createChild() fails.

@briancavalier

This comment has been minimized.

Show comment
Hide comment
@briancavalier

briancavalier Jul 16, 2013

Member

Directly using the undocumented APIs, like Scope, in lib is not currently supported. While we are moving in a direction that will allow more programmatic use, the APIs are still too fluid. Wire's primary API is the wire function, amd plugin, wire plugins, and the declarative spec format.

Member

briancavalier commented Jul 16, 2013

Directly using the undocumented APIs, like Scope, in lib is not currently supported. While we are moving in a direction that will allow more programmatic use, the APIs are still too fluid. Wire's primary API is the wire function, amd plugin, wire plugins, and the declarative spec format.

@AnAppAMonth

This comment has been minimized.

Show comment
Hide comment
@AnAppAMonth

AnAppAMonth Jul 16, 2013

Try to wire this spec, and you encounter an error:

{
    myscope: {
        mycontext: {
            wire: {
                spec: 'myspec'
            }
        }
    }
}

Try to wire this spec, and you encounter an error:

{
    myscope: {
        mycontext: {
            wire: {
                spec: 'myspec'
            }
        }
    }
}
@briancavalier

This comment has been minimized.

Show comment
Hide comment
@briancavalier

briancavalier Jul 16, 2013

Member

Have a look at #100 for more thoughts on programmatic APIs. Feel free to post your thoughts there! That's just the tip of the iceberg, and exposing more pieces, like Scope (or whatever Scope ends up being in v0.11), will probably be in the plan as well.

Member

briancavalier commented Jul 16, 2013

Have a look at #100 for more thoughts on programmatic APIs. Feel free to post your thoughts there! That's just the tip of the iceberg, and exposing more pieces, like Scope (or whatever Scope ends up being in v0.11), will probably be in the plan as well.

@briancavalier

This comment has been minimized.

Show comment
Hide comment
@briancavalier

briancavalier Jul 16, 2013

Member

Haha, yep, that breaks. Sorry I misunderstood your original post. Including test cases helps a lot. Thanks!

Member

briancavalier commented Jul 16, 2013

Haha, yep, that breaks. Sorry I misunderstood your original post. Including test cases helps a lot. Thanks!

@briancavalier

This comment has been minimized.

Show comment
Hide comment
@briancavalier

briancavalier Jul 16, 2013

Member

@halfninety I think there is a fix in master now, in 2371067. Could you try it and let me know if that works for you?

Member

briancavalier commented Jul 16, 2013

@halfninety I think there is a fix in master now, in 2371067. Could you try it and let me know if that works for you?

@AnAppAMonth

This comment has been minimized.

Show comment
Hide comment
@AnAppAMonth

AnAppAMonth Jul 17, 2013

Is the change in wirePlugin.js intended to fix a bug that causes components inside sub-contexts to be created multiple times? I just discovered the bug and located its source, and your change in wirePlugin.js fixes that.

Anyway, what's your purpose of mixing the sub-context's spec into the sub-context's instances dictionary? I figure you might remove this now.

Is the change in wirePlugin.js intended to fix a bug that causes components inside sub-contexts to be created multiple times? I just discovered the bug and located its source, and your change in wirePlugin.js fixes that.

Anyway, what's your purpose of mixing the sub-context's spec into the sub-context's instances dictionary? I figure you might remove this now.

@briancavalier

This comment has been minimized.

Show comment
Hide comment
@briancavalier

briancavalier Jul 17, 2013

Member

Is the change in wirePlugin.js intended to fix a bug that causes components inside sub-contexts to be created multiple times? I just discovered the bug and located its source, and your change in wirePlugin.js fixes that.

I've not noticed this problem, but it sounds pretty serious. I'll check on it. It could be related to the following bug, which I believe is fixed (I'm planning to release 0.10.1 today with the fix):

Anyway, what's your purpose of mixing the sub-context's spec into the sub-context's instances dictionary? I figure you might remove this now.

This was definitely a bug, and should be fixed in 2371067. It's also fixed in dev.

Member

briancavalier commented Jul 17, 2013

Is the change in wirePlugin.js intended to fix a bug that causes components inside sub-contexts to be created multiple times? I just discovered the bug and located its source, and your change in wirePlugin.js fixes that.

I've not noticed this problem, but it sounds pretty serious. I'll check on it. It could be related to the following bug, which I believe is fixed (I'm planning to release 0.10.1 today with the fix):

Anyway, what's your purpose of mixing the sub-context's spec into the sub-context's instances dictionary? I figure you might remove this now.

This was definitely a bug, and should be fixed in 2371067. It's also fixed in dev.

@briancavalier

This comment has been minimized.

Show comment
Hide comment
@briancavalier

briancavalier Jul 17, 2013

Member

components inside sub-contexts to be created multiple times

Do you have a test case for this one? I tried to reproduce it in an earlier version of dev (before the fix for the original problem in this issue) and wasn't able to. As you say, it does seem to work after the original fix as well.

Thanks!

Member

briancavalier commented Jul 17, 2013

components inside sub-contexts to be created multiple times

Do you have a test case for this one? I tried to reproduce it in an earlier version of dev (before the fix for the original problem in this issue) and wasn't able to. As you say, it does seem to work after the original fix as well.

Thanks!

@AnAppAMonth

This comment has been minimized.

Show comment
Hide comment
@AnAppAMonth

AnAppAMonth Jul 17, 2013

I created a new issue #128. I almost finished writing it before I discovered you already fixed it..

I created a new issue #128. I almost finished writing it before I discovered you already fixed it..

@briancavalier

This comment has been minimized.

Show comment
Hide comment
@briancavalier

briancavalier Jul 17, 2013

Member

Awesome, I really appreciate the amazing level of detail in that issue!!! I'll followup there.

Member

briancavalier commented Jul 17, 2013

Awesome, I really appreciate the amazing level of detail in that issue!!! I'll followup there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment