-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
Remove DynamicDependency from ValueType in browser builds #47909
Comments
I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label. |
Tagging subscribers to 'arch-wasm': @lewing Issue Details
|
Tagging subscribers to 'size-reduction': @eerhardt, @SamMonoRT, @marek-safar, @tannergooding Issue DetailsBy removing global
|
The landscape changed and the In scenarios which use interop code directly, like projects with Console, HTTP client or WebSocket client naturally use JSFunctionBinding from the generated But we still have the legacy interop code, which depends on many things in the |
We discussed this with @vitek-karas. We could add Second option (not prefered) is to add linker root into XML like here |
Just to clarify the proposed approaches:
|
Unfortunately |
And the XML descriptor doesn't help either for the same reason. :( |
We could add conditional XML descriptor to corlib, but it's not much better than the current state. |
Could we add the dependency to source generated code? |
The only thing I can think of is for the source generator to add something like It's also questionable if the goal should be to root all assemblies which have JSExport in them - what if I use library which has that, but I don't need that part... I would have no way to get rid of such assembly. ASP.NET has something called application parts (https://learn.microsoft.com/en-us/aspnet/core/mvc/advanced/app-parts?view=aspnetcore-6.0) which is basically a way to register components with the app. I vaguely remember that there some build-time component associated with these so that the components are made part of the app. In this case there's no need to register the assemblies (as they're going to be refereced as packages), but there is still a need to tell the system somehow to enable JS interop on them. @sbomer for additional ideas... |
My understanding is that there's a legacy scenario where It sounds like |
The problem is if this happens across assemblies. There's nothing in managed world which would express the dependency between the app and MyJSHelper. And source generators won't help, because when it runs on MyJSHelper it sees the JSExport but can't "root" the assembly itself (you're just building a library). And when you run it on the app - there's no JSExport anywhere, so nothing the generator can react to. |
I see, then I think the linker fundamentally doesn't have insight into which JSExports may be called. The only automated way I can think of to solve this is for a JS bundler to determine which JS entry points are used, then for a component of our tooling to map these to the required exports (and root those when linking). But that requires complicated cross-language tooling interop - especially if the interop goes in both directions, where managed code trimming could reduce the set of used JS code. I think the best solution for now is to require the user to manually preserve the required exports, for example using |
As Vitek said, it could get as bad as the whole assembly could be trimmed because of that. But user code could have the same issue, if they don't have application managed code reference to a assembly with https://github.com/maraf/dotnet-wasm-react The managed tooling doesn't have easy way how to know which So, I think that |
Disabling trimming in the assembly is probably too much - all you need is to keep all the JSExport methods (and everything they depend on), the rest of the assembly should be trimmed as anything else. I don't think we have anything in the linker which would do this currently. Even I still think that assemblies should not be able to "root" themselves in the app - it's kind of a bad precedent. But the only other approach would require some kind of build-time tool which scans assemblies for the JSExport attribute and adds them to the "Roots" some other way. Which is equally bad. I think this is up to you @pavelsavara to define the desired behavior - is you think that the best approach is to automatically root all JSExport methods (and probably without a way for the user to revoke that decision), then we will need to introduce some new feature into the linker to do this - either dotnet/linker#2991, or something else. |
Let pause and have this issue collect community feedback for few months. I have no data if it really happens in user apps. |
How does the |
Yep, I agree this is not JS problem only but AFAIK we don't have a reliable way how to keep method (or even field) even if it's not used beyond I'd also not mix this with the concept of how to trim libraries in general, which is a similar but different problem. |
Currently native hosting and COM are not supported with trimming, so we didn't even try to solve this. |
Tagging subscribers to this area: @dotnet/area-system-runtime Issue DetailsBy removing global
|
By removing global
[DynamicDependency(DynamicallyAccessedMemberTypes.PublicMethods, "System.Runtime.InteropServices.JavaScript.JSFunctionBinding", "System.Runtime.InteropServices.JavaScript")]
attribute from System.ValueType we could keep only marshaling support for types that are used. The attribute needs to be used today because there is no reference to the assembly from managed code.The text was updated successfully, but these errors were encountered: