Provide ability to add project references #1318
Conversation
This change updates NodejsProjectNode to detect if the UWP project flavor is active to provide support for project references. Alternative approach could have been to make NodejsProjectNode public (so that I could reference it from the UWP project system) but that would cause a huge changes as opposed to this less intrusive change.
|
I don't know much about this area of the code. @zooba, can you please take a quick look at the changes in common? Also @munyirik, can you provide an overview of how you tested for regressions and how to test the new functionality? This new functionality should only effect NTVS IOT projects, not regular NTVS flows, correct? Thanks. |
|
I ran the Add* , Project* , and Build* tests in this repo. Most passed and the failures were unrelated to this change. I also created projects with the NTVS templates to manually check that the references node is not included. That said, I DO need to add a test that checks this. I'll do that. I also wanted to mention that the the changes in ProjectNode are to show the new style reference manager. Without the change it will look like this: With the change it looks like this: The functionality can be tested by cloning https://github.com/munyirik/ntvsiot and running an experimental instance of the project. I'll also be adding a test there as well. |
|
Is ProjectTests.NoReferences supposed to be testing this? It passes whether the reference node is present or not. |
|
@mjbvz to answer you last question - yes, it will only affect NTVS IoT projects. |
| private IVsReferenceProviderContext CreateFileReferenceProviderContext(IVsReferenceManager mgr) { | ||
| var context = mgr.CreateProviderContext(VSConstants.FileReferenceProvider_Guid) as IVsFileReferenceProviderContext; | ||
|
|
||
| context.BrowseFilter = "Component Files" + "\0*.winmd\0\0"; |
There was a problem hiding this comment.
This should use AddReferenceExtensions (or some updated virtual) instead of hard coding the list of extensions that are supported
| // Add the WebPI page | ||
| tabInit[3].dwSize = (uint)Marshal.SizeOf(typeof(VSCOMPONENTSELECTORTABINIT)); | ||
| tabInit[3].guidTab = typeof(WebPiComponentPickerControl).GUID; | ||
| tabInit[3].varTabInitInfo = 0; |
There was a problem hiding this comment.
Is there any way to keep port the WebPI component picker over and keep it present?
There was a problem hiding this comment.
As discussed offline, I'll keep WebPI out for this commit
| } | ||
|
|
||
| uint pX = 0, pY = 0; | ||
| Array IVsReferenceManagerUser.GetProviderContexts() { |
There was a problem hiding this comment.
Seems like this should call into a protected virtual version of this so subclasses can customize the available providers
| private __VSREFERENCECHANGEOPERATIONRESULT AddReferences(IVsReferenceProviderContext context) { | ||
| var addedReferences = Enumerable.Empty<VSCOMPONENTSELECTORDATA>(); | ||
|
|
||
| if (context.ProviderGuid == VSConstants.ProjectReferenceProvider_Guid) { |
There was a problem hiding this comment.
These if checks should also be pulled into a protected virtual so that derived implementations can check for additional provider guids and then call the base class for the defaults
| private __VSREFERENCECHANGEOPERATIONRESULT RemoveReferences(IVsReferenceProviderContext context) { | ||
| var removedReferences = Enumerable.Empty<ReferenceNode>(); | ||
|
|
||
| if (context.ProviderGuid == VSConstants.ProjectReferenceProvider_Guid) { |
Also moves checks for adding/removing reference providers to virtual methods as well.
|
@DinoV any other comments? |
|
@munyirik Since there were no other follow up comments, I've merged this is. The change will be in the next dev build, but will not go into VS15 RC since we have already branched that off. Thanks again for the contribution and follow up here! |
|
Thanks! When will the next dev build be released? |
|
Let me take in #1343 first and then I can get out another dev build this afternoon. One other point: right now the devbuilds only work for VS2015 though, we're still working on a proper good dev build story for VS15. The current steps required are a bit too involved. |
|
That works for me since the IoT extension only works with VS2015 currently. Did you have to do major changes to support VS15? I'll need to support it eventually and I'm curious how much work it will involve. |
|
It depends. Not many code changes were required to get NTVS up and running on VS15 itself, and most of our work has been to support new features and improvements that other teams are bringing into VS15. We also had to do a lot of work to get NTVS shipping as a VSIX package in a VS workload. If you are already shipping as a VSIX, what you are most likely to run into with the new install experience is missing components. The Node.js workload in VS15 may not pull in all the components that your IoT extension requires. |
4 participants
This change is needed for the https://github.com/ms-iot/ntvsiot project (the subtype of NTVS for targeting UWP) to allow adding references to projects.
I couldn't find a way to only change the subtype in a less intrusive way. If there is, please let me know.
The change in ProjectNode.cs will not affect NTVS functionality since it doesn't make use of the reference manager. In NodejsProjectNode.cs I added a condition to check if the UWP project subtype is active to decide whether to add the reference node.