Skip to content

Remove DynamicDependency from ValueType in browser builds #47909

Open

Description

@marek-safar

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.

Activity

  1. ghost added
    untriagedNew issue has not been triaged by the area owner
    on Feb 5, 2021
  2. removed
    untriagedNew issue has not been triaged by the area owner
    on Feb 5, 2021
  3. added
    size-reductionIssues impacting final app size primary for size sensitive workloads
    on Feb 5, 2021
  4. added this to the 6.0.0 milestone on Feb 5, 2021
  5. modified the milestones: 6.0.0, 7.0.0 on Jul 7, 2021
  6. self-assigned this
    on Jan 31, 2022
  7. pavelsavara commented on Jul 11, 2022

    @pavelsavara
    Member

    The landscape changed and the ValueType now points to public type JSFunctionBinding which points to internal JavaScriptExports. The situation is bit better because the JavaScriptExports contains only methods which need to be reachable from JavaScript.

    In scenarios which use interop code directly, like projects with Console, HTTP client or WebSocket client naturally use JSFunctionBinding from the generated [JSImport] code.

    But we still have the legacy interop code, which depends on many things in the JavaScriptExports class. So at least for now, I don't think we could get rid of DynamicDependency on ValueType.

  8. removed this from the 7.0.0 milestone on Jul 11, 2022
  9. 14 remaining items

  10. vitek-karas commented on Oct 31, 2022

    @vitek-karas
    Member

    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 AssemblyMetadata("IsTrimmable", "false") is undefined as far as I know - dotnet/linker#2991.

    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.

  11. pavelsavara commented on Oct 31, 2022

    @pavelsavara
    Member

    Let pause and have this issue collect community feedback for few months. I have no data if it really happens in user apps.

  12. pavelsavara commented on Oct 31, 2022

    @pavelsavara
    Member

    How does the [UnmanagedCallersOnly] deal with the same issue ? Or any [ComVisible(true)] export ?

  13. marek-safar commented on Oct 31, 2022

    @marek-safar
    ContributorAuthor

    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 DynamicDependency which is not suitable here. Our workaround is not to trim user assembly at all where such scenario with JSExport occurs most often.

    I'd also not mix this with the concept of how to trim libraries in general, which is a similar but different problem.

  14. vitek-karas commented on Oct 31, 2022

    @vitek-karas
    Member

    How does the [UnmanagedCallersOnly] deal with the same issue ? Or any [ComVisible(true)] export ?

    Currently native hosting and COM are not supported with trimming, so we didn't even try to solve this.

  15. modified the milestones: 8.0.0, Future on Jan 3, 2023
  16. modified the milestones: Future, 9.0.0 on Nov 22, 2023
  17. modified the milestones: 9.0.0, Future on Jan 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

arch-wasmWebAssembly architecturearea-System.Runtimesize-reductionIssues impacting final app size primary for size sensitive workloads

Type

No type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions