Should we change package-lock.json to yarn-lock.json and use yarn workspaces?

[aaronamm]

I found that OrchardCore.Apis.GraphQL use Webpack and when it build we will have node packages inside that module.
This means if later we add more Webpack in other modules, we will have many duplicated Node packages inside each module.

It would be nice if we could add yarn workspace and centralize installation of node packages to the root folder of OrchardCore project.

In addition after we have yarn workspaces, we can install, update, build Node project in all modules with a single command. We don't need to navigate to each project and run a command.

[Skrypt]

I like the idea of workspaces. Though, I don't think it is necessary as everything works fine as it is right now. If you want to do it, you can create a Pull Request that will get reviewed to make sure everything is still working. I personally don't think the effort is worth it since this is not going to build assets better in the end. The advantages are really low vs the time it will take to do.

[aaronamm]

@Skrypt Thanks for prompt reply.
I understand your point. However, if we deploy OrchardCore to Azure App and we can have many node projects, centralize node packages with yarn workspaces can save disk space for us to use it with other App Service in the same plan.

To summarize, I will create a PR but I need to inform you that workspace does not work with a normal App Service deploy e.g. Kudu. This is because lake of file permission that workspaces use to link files under the hood. To make it work, I think we need to deploy as Azure App Service container.

[aaronamm]

@Skrypt
Here is my PoC to make sure we can use Yarn Workspace when deploy to Azure App Service
https://github.com/codesanook/CoreMvcAppService/blob/develop/.github/workflows/deploy-app-service.yml

[Skrypt]

It is worth investigating if building our Assets should be done from CI. That would avoid a lot of merge issues happening right now because sometimes assets rebuild themselves based on if you use a different version of npm which will also change your package.lock.json files.

We currently are pre-building these assets so we don't build them again in the CI. I can see an issue here where the CI could rebuild these assets differently based on different reasons. But like I said, it's worth investigating for now.

[aaronamm]

@Skrypt, could you please let me if you need someone to work on build TS/Sass after you have done with investigating, I am happy to help working on this.

[Skrypt]

Will do, right now the focus is on releasing 1.0 these things can wait a little. I will let @sebastienros triage this on next meeting and see from there. We need to discuss it first.

[aaronamm]

@Skrypt Thanks for prompt reply. Can't wait for the 1.0 official release. 🙏🙏🙏

[aaronamm]

@Skrypt just for my curiosity, if we have a simple Node project in each OC modules, can yarn workspace help us to maintain updating NPM package in each project easily like in this video? https://egghead.io/lessons/yarn-use-yarn-up-to-update-dependencies-in-a-yarn-workspace.

We can also use yarn workspaces run build to build all Node projects in each OC module with a single command.

[Skrypt]

I don't think the issue is there. The problem is that we have custom assets in each modules which needs to be built only when a change is found in one of these files. With the gulpfile.js pipeline we harvest for each modules assets using the Assets.json file standing in each of thems. So using yarn workspace here would only be only usefull for assets that are retrieved from NPM. We also have modules and themes that uses gulp and others that uses webpack so changing things in the assets build while we are close to release OC 1.0 is a really bad call. I keep the issue opened because it needs to be discussed more and analyzed/evaluated in details. Also, why should we choose yarn over any other alternatives out there?

[aaronamm]

@Skrypt Thanks for the information and totally agree with you that it's too late call when closing to RC 1.0.

[Piedone]

With the discussion under #15740, I think this can be closed, right?

[github-actions]

It seems that this issue didn't really move for quite a while despite us asking the author for further feedback. Is this something you'd like to revisit any time soon or should we close? Please reply.

[github-actions]

Closing this issue because it didn't receive further feedback from the author for very long. If you think this is still relevant, feel free to reopen it with the requested details.