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

Add ability to inject into module initializer #165

Merged
merged 3 commits into from
Oct 5, 2019

Conversation

hymccord
Copy link
Contributor

@hymccord hymccord commented Oct 5, 2019

This PR adds support for library initialization. It looks for the ModuleDefinition's type and then creates a static constructor if one isn't already found.

The .cctor is called when the module is loaded (not necessarily on assembly load).

This is mostly done. I need to write some unit tests for it but it doesn't seem AssemblyGenerator supports weaving in settings so by default AutoInit is true which means AutoLibraryInit is off. Maybe something to look into? Turning it on does mangle a lot of the unit tests because DI.Init tries to get called twice. Hey, at least I know it's working 👍

I'd also like to discuss the setting name. I tried to come up with something good but it might not be the best while also debating whether to turn the AutoInit from a bool into an enum. Maybe like

enum AutoInitMode
{
   None
   EntryPoint
   ModuleLoad
}

That would be a breaking change so I decided not to for now.

I've never used AutoDI but it does look neat! Once (and if) this PR is accepted, I'd be happy to update the relevant wiki related to configuration as well!

Related: #154

@Keboo
Copy link
Owner

Keboo commented Oct 5, 2019

Wow, really nice work!
Regarding the setting name. I think I like the option of converting AutoInit into an enum as you suggest, though I would drop "Auto" from the enum name.

enum InitMode
{
   None
   EntryPoint
   ModuleLoad
}

I realize this will be a breaking changes, but I have no problem rev'ing the major version to add in this feature.

As for the generator not supporting the SettingsAttribute. I have been aware of it, and TBH I am not overly happy with how the AssemblyGenerator works. There is a bit of a hack I put in to handle this case where you can specify some arbitrary raw content to get added. This has been my way of testing the various settings.

@hymccord
Copy link
Contributor Author

hymccord commented Oct 5, 2019

Okay, I think that's everything! Thank you for the feedback.

Summary of breaking changes

  • SettingsAttribute.AutoInit name changed to SettingsAttribute.InitMode
  • SettingsAttribute.InitMode is now a InitMode enum with a default of InitMode.EntryPoint so you can specify what type of auto initialization you want but keep the existing behavior of EntryPoint.

@Keboo Keboo merged commit 02e7fbd into Keboo:master Oct 5, 2019
@Keboo
Copy link
Owner

Keboo commented Oct 5, 2019

Great work. Thank you

@hymccord hymccord deleted the features/library-support branch October 5, 2019 19:35
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