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

[rush] Deploy command is preserving workspace:* entries #2723

Open
thelinuxlich opened this issue Jun 2, 2021 · 9 comments
Open

[rush] Deploy command is preserving workspace:* entries #2723

thelinuxlich opened this issue Jun 2, 2021 · 9 comments
Labels
needs design The next step is for someone to propose the details of an approach for solving the problem needs more info We can't proceed because we need a better repro or an answer to a question
Projects

Comments

@thelinuxlich
Copy link

thelinuxlich commented Jun 2, 2021

Summary

I'm confused. I thought rush deploy was supposed to create a generic artifact to be deployed in restrict environments, like Google Cloud which doesn't support NPM. However, when I run rush deploy I can see the package.json of the deployed projects in common/deploy/packages/<project_name> are still using the "workspace:*" syntax. Is this a bug or intended?

Repro steps

Expected result: Generated package.json in deploy folder without "workspace:*" syntax

Actual result: Generated package.json still preserving exclusive PNPM syntax

Standard questions

Please answer these questions to help us investigate your issue more quickly:

Question Answer
@microsoft/rush globally installed version?
rushVersion from rush.json?
useWorkspaces from rush.json?
Operating system?
Would you consider contributing a PR?
Node.js version (node -v)?
@iclanton
Copy link
Member

iclanton commented Jun 3, 2021

@octogonz - is this by design?

@octogonz
Copy link
Collaborator

octogonz commented Jun 3, 2021

Alisson asked the same question in this Zulip thread. I'm trying to get more details about what it would be used for.

(It's not difficult to implement, but it's unclear why someone would need it.)

@octogonz octogonz added the needs more info We can't proceed because we need a better repro or an answer to a question label Jun 3, 2021
@thelinuxlich
Copy link
Author

In limited cloud environments like Google App Engine you don't have the luxury of PNPM and the environment strips the node_modules folder and forces a npm install, which doesn't recognize the workspace syntax in package.json.

@octogonz
Copy link
Collaborator

This was my question from Zulip:

Okay, but if you are not installing them, then what tool would read the file:../foo version specifiers? I'm trying to understand why there is any problem with keeping workspace:* in the package.json files. Is there any process in your setup that reads the deployed package.json files and has trouble with workspace:*? Or is this a purely cosmetic request?

@thelinuxlich could you give us some insight why you are requesting this feature?

@thelinuxlich
Copy link
Author

It's what I said above, limited cloud environments like Google force the npm install after transferring the files.

@iclanton iclanton added this to Needs Investigation in Bug Triage Jun 24, 2021
@mralbobo
Copy link

mralbobo commented Oct 25, 2023

Just in case this accomplishes something, some more details as requested.

Google cloud functions and firebase cloud functions (and probably other google cloud things) have a fairly unique behavior where the build/ deploy workflow is

  • build on your machine
  • run their deploy script which
    • deleted node_modules
    • zips up the folder
    • uploads it to their server
    • which runs npm install

And nothing is configurable in any useful way, practically speaking this means that any code the cloud function needs to execute either needs to be a local file (not in node_modules) or be installable by npm install.

Of course, this is 100% a dumb behavior from google cloud we're trying to work around, but there's definitely one of several configurations this project could support which would help.

Here's an ungodly long issue on the firebase-cli repo firebase/firebase-tools#653 that describes the issue and the various workarounds people people have come up with.

For the most part it boils down to one of two solutions with varying levels of complexity and re-usability.

  1. put your libraries in a private registry so the google cloud engine can install them
    For this solution, it would be super useful if there was a way to configure rush deploy to convert the workspace:* references to a version number. This has the limitation where you'd only be able to use published versions of packages

  2. package up the libraries into tar's and ship them with the function
    Basically this involves something like rush publish --pack to create tars, then rewriting the workspace:* to be file:// references to those tars. Because the tars are not in node_modules anymore, and npm can install from file urls, the files are uploads as part of the deploy. And put back in the right place by cloud builder. So similarly, a config to do that more automatically would also be handy.

Sooo... there's the usecase, basically working around poor design from google

@0x80
Copy link

0x80 commented Mar 31, 2024

You might want to look into isolate-package and firebase-tools-with-isolate (which integrates the isolate process so that it only happens on deployment and you can still run the emulators as usual with HMR).

They were developed specifically to solve the challenges with Firebase deployments (which I wrote about here), but isolate-package is generic enough to be useful for other scenarios like creating docker images. I use it for Cloud Run deployments.

Both now support Rush monorepos. I have only tested it with PNPM because I don't use Rush myself, but I'm fairly confident that things will work with NPM and Yarn as well.

This is the example repo I've tested it on.

@octogonz
Copy link
Collaborator

octogonz commented Apr 1, 2024

To answer the original question in this thread:

I thought rush deploy was supposed to create a generic artifact to be deployed in restrict environments, like Google Cloud which doesn't support NPM. However, when I run rush deploy I can see the package.json of the deployed projects in common/deploy/packages/<project_name> are still using the "workspace:*" syntax. Is this a bug or intended?

Repro steps

Expected result: Generated package.json in deploy folder without "workspace:*" syntax

Actual result: Generated package.json still preserving exclusive PNPM syntax

The inclusion of workspace:* syntax is not a bug; it is intended. The design of rush deploy is to create a folder (or zip archive) that can be copied to a server machine, without need to invoke pnpm install or npm install. The extracted node_modules folder tree is expected to resolve your project's dependencies the same as if it had been installed from the NPM registry. Although there is no workspace, the presence of workspace:* should not cause any trouble, because the NodeJS module resolution algorithm does not look at package.json entries; instead it traverses the node_modules files and directory relationships.

In limited cloud environments like Google App Engine you don't have the luxury of PNPM and the environment strips the node_modules folder and forces a npm install, which doesn't recognize the workspace syntax in package.json.

To avoid confusion, perhaps we should retitle this GitHub issue or create a new GitHub issue. If I understand correctly, the request is:

Could rush deploy produce an output that can be installed using npm install instead of copying files or extracting a .zip archive.

Is that correct? For example, one idea might be to create an NPM package tarball that with (1) NO package.json dependencies and (2) an "install" lifecycle script that extracts the .zip archive and then runs something equivalent to node create-links.js create.

Would that meet the requirements for Google App Engine? (Any other proposals?)

@octogonz octogonz added the needs design The next step is for someone to propose the details of an approach for solving the problem label Apr 1, 2024
@0x80
Copy link

0x80 commented Apr 2, 2024

Could rush deploy produce an output that can be installed using npm install instead of copying files or extracting a .zip archive.

Isolate-package can do this. It has a forceNpm option, to construct an NPM installable output from a PNPM or YARN monorepo, but I wouldn't recommend it, because the current solution (using Arborist) is sub-optimal. PNPM is the happy path IMO and as I said Firebase and Cloud Run support it for sure.

For PNPM the isolated output remains a workspace, because this is how PNPM likes to have things. When I tried to convert the workspace:* entries in the manifest to link:, I discovered that PNPM doesn't install links to local files like NPM and Yarn do.

So for PNPM the internal dependencies are moved to the isolated output folder, and are preserved as a workspace, and effectively it still is a monorepo setup but can be installed in a cloud environment as-is.

The outputs for Yarn and NPM use link: to declare their internal dependencies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs design The next step is for someone to propose the details of an approach for solving the problem needs more info We can't proceed because we need a better repro or an answer to a question
Projects
Status: Needs Investigation
Bug Triage
  
Needs Investigation
Development

No branches or pull requests

5 participants