Skip to content

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

Merged
mpiroc merged 2 commits intomasterfrom
pirocchi/anchor
Sep 7, 2018
Merged

Force generated assemblies to reference jsii-only dependencies.#216
mpiroc merged 2 commits intomasterfrom
pirocchi/anchor

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