-
Notifications
You must be signed in to change notification settings - Fork 636
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
[Draft] Defer loading of workspace referenced packages until workspace open #11502
base: master
Are you sure you want to change the base?
Conversation
@@ -91,7 +92,7 @@ public PackageLoader(IEnumerable<string> packagesDirectories) | |||
if (packagesDirectories == null) | |||
throw new ArgumentNullException("packagesDirectories"); | |||
|
|||
this.packagesDirectories.AddRange(packagesDirectories); | |||
//this.packagesDirectories.AddRange(packagesDirectories); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't load all packages, just the ones in the standard lib.
foreach (var info in packageDependencies) | ||
{ | ||
// if workspace package dependency is present in downloaded packages, load it. | ||
if (info.Name == discoveredPkg.Name && info.Version.ToString() == discoveredPkg.VersionName) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Collect all downloaded packages that match with the ones referenced in the workspace.
@@ -1693,6 +1693,8 @@ private bool OpenJsonFileFromPath(string fileContents, string filePath, bool for | |||
OnComputeModelDeserialized(); | |||
|
|||
SetPeriodicEvaluation(ws); | |||
|
|||
CurrentWorkspace = ws; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update the current workspace here, after the VM is re-initialized and all libs/packages are reloaded. This will trigger the property changed event for CurrentWorkspace
at the right time for the DynamoPackages
extension to react to it and load packages for that new workspace.
@@ -1771,7 +1773,7 @@ private void OpenWorkspace(WorkspaceModel ws) | |||
} | |||
|
|||
AddWorkspace(ws); | |||
CurrentWorkspace = ws; | |||
//CurrentWorkspace = ws; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hope this doesn't cause any side-effects. As noted above I have moved this so that it happens a little later in the workspace opening sequence.
if (PackageLoader != null && ws is HomeWorkspaceModel model) | ||
{ | ||
PackageLoader.OnWorkspaceRead(model.NodeLibraryDependencies, | ||
ReadyParams.StartupParams.PathManager.PackagesDirectories); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Called once a new workspace is opened.
Purpose
This is an attempt to load packages at the time of opening a workspace by looking at the workspace references and only loading the necessary packages from the ones that are downloaded.
Declarations
I have not been able to get this to work after spending a few days as it seems it will require bigger changes to the engine. However, this PR should serve as a reference for future work.Finally got deferred loading to work - was able to load packages referenced by a workspace at the time of opening it.Check these if you believe they are true
*.resx
files