-
Notifications
You must be signed in to change notification settings - Fork 70
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(fix): import mapping resource overlap #557
base: main
Are you sure you want to change the base?
wip(fix): import mapping resource overlap #557
Conversation
A local resource should never be an import - and imports always need to provide external resources that correspond to the correct imported resource type. So this should be less about import mapping and more about whether the resource is imported or not. That is, the approach in this PR doesn't sound correct to me - it would be good to dig in further to find the root cause fix here. |
Do we have a test case that replicates the issue? |
This failure actually is a part of the test cases that was commented out (I'm assuming because something was wrong): Line 564 in 46e729c
Once I tried to uncomment that bit of the test, I ran into this issue and trying to trace it down, going through the generated code. What I ran into was that I described here. It was a bit ago but I tried to write enough to describe what I thought was happening. Definitely open to ideas on what the problem could actually be here! |
A good starting point would be to uncomment that test in this PR and see if the tests are passing. |
Sure, that's reasonable! just pushed, will rebase to main as well ( [EDIT] BTW, this is the error I was seeing (I don't think it's changed):
|
This commit attempts to fix the case of an import map that perfectly overlaps an existing component resource. When a resource that exists in a component is overriden with an import map, the interface can change arbitrarily. Signed-off-by: Victor Adossi <[email protected]>
Signed-off-by: Victor Adossi <[email protected]>
Signed-off-by: Victor Adossi <[email protected]>
19cf57a
to
5721251
Compare
This commit attempts to fix the case of an import map that perfectly overlaps an existing component resource.
When a resource that exists in a component is overriden with an import map, the interface can change arbitrarily.
This handle failure seems to actually be much deeper... Where I've landed is that resources are broken when import mapped over.
I've been able to figure out that the import map'd resource that overlaps with the local one is being interpreted as a host & imported & owned resource.
They basically are being detected as owned when they should not be -- while it is possible to have a
ResourceData::Host
that is imported and owned, in this case the resource is actually coming from the import map (it's not the same resource as the component created/is in the WIT).I can't tell if the right move here is to:
I'm thinking it might be a componentize problem because the function in the WASM has no idea about the import mapping above, and is probably trying to lower using the definitions in the component (that don't match the import map).
The first issue was attempting to remove the resource that wasn't actually successfully created which I found a way to work around (that maybe makes sense), but I'm finding that while you can create the resource, you can't consume it (i.e. from
consumeBar
) inside the component.