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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Force generated assemblies to reference jsii-only dependencies. #216

Merged
merged 2 commits into from
Sep 7, 2018

Conversation

mpiroc
Copy link
Contributor

@mpiroc mpiroc commented Sep 7, 2018

When a NuGet package is restored, all of its listed dependencies are restored (whether or not they are actually used). But when a .NET assembly is built, if it does not reference any .NET types from the dependency, the compiled .NET assembly will not reference the dependency .NET assembly.

At runtime, jsii-dotnet-runtime loads JSII dependencies by looking through the .NET assembly's references. If the dependency does not appear in references, it won't be found.

This breaks the following scenario:

  • A JSII assembly my-package depends on my-dependency.
  • In the TypeScript source, my-package requires symbols from my-dependency. Therefore the NuGet package MyPackage lists MyDependency as a dependency.
  • However, the generated .NET source for MyPackage does not reference any symbols from MyDependency.

At runtime, the first time a type from MyPackage is used, jsii-dotnet-runtime will search for MyPackage's dependencies, and ask jsii-runtime to load each of them (i.e. my-dependency) before attempting to load my-package. But due to the missing assembly reference, it won't be able to find my-dependency. So when the javascript code for my-package attempts to require('my-dependency'), it will fail.

To work around this issue, for each package, we generate a type MyNamespace.Internal.DependencyResolution.Anchor that references the Anchor from each of its dependencies. This way all listed dependencies will show up in the .NET assembly's references, avoiding this issue.


return Path.Combine(path, "Internal", "DependencyResolution", "Anchor.cs");
}

static string GetTypeFilePath(string dotnetPackage, string dotnetNamespace, string dotnetType)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_ private (be obvious)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm find with adding explicit access modifiers where they are currently implied, but currently the codebase omits all redundant access modifiers. If we change this, we should change it across the whole codebase.

Created #217 to track this.

@mpiroc mpiroc merged commit cf62773 into master Sep 7, 2018
@mpiroc mpiroc deleted the pirocchi/anchor branch September 7, 2018 20:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants