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

[Poll] Preference on how to filter workspaces #160

Closed
ruyadorno opened this issue Jun 10, 2020 · 10 comments
Closed

[Poll] Preference on how to filter workspaces #160

ruyadorno opened this issue Jun 10, 2020 · 10 comments

Comments

@ruyadorno
Copy link
Contributor

ruyadorno commented Jun 10, 2020

This is just a quick poll to collect feedback on the overall preference from the community on how to filter workspaces when running npm commands, in case you have any technical concerns about these or if you want to bring something up to discussion please do so in the original RFC instead.

Given a nested package foo which is declared as a workspace for a top-level workspaces-enabled project, how would you prefer to run commands on it (following examples using test command):

1️⃣ Positional argument with one-char alias: npm workspace foo test or npm w foo test
2️⃣ Positional argument but shorter default name: npm ws foo test or npm w foo test
3️⃣ Special character (operator-like): npm :foo test
4️⃣ Named argument: npm --workspace=foo test or npm -w=foo test
🔀 Something else? Comment to suggest different alternatives




@ljharb
Copy link
Contributor

ljharb commented Jun 10, 2020

  • short names should only be aliases for nice and verbose long ones; ie, ws can be short for workspace but the name of the command is "workspace" and they have to do the same thing
  • If the semantic is only "provide a single subpackage/workspace" then --workspace= (on any npm or npx command) makes sense to me. however that's way less useful than "provide a glob pattern that matches 0-n workspaces", which would need a different name, like "scope" or "filter" or something.

@ruyadorno
Copy link
Contributor Author

ruyadorno commented Jun 10, 2020

Somehow I failed to make the polls bot work here, I'll try again later.

@ljharb that's good feedback, please bring it to the original RFC instead - note: what we have in mind for now to solve the glob story/requirement is using that Group/Alias feature described there in the original RFC 😊

@ruyadorno
Copy link
Contributor Author

oof after some back and forth it looks like the poll is on! 😅 thanks @darcyclarke!

@wesleytodd
Copy link

I think the standard -- argument is also good because you can accept a comma separated list and it is a commonly understood interface.

@fed135
Copy link

fed135 commented Jun 11, 2020

Probably not as solid an argument, but I think there's some value to keeping it close to what already exists out there. For better or worse it's what the community came to adopt.

Yarn has two forms to either run on a single workspace or multiple workspaces, while lerna has a single syntax that runs commands on all packages by default, but allows a --scope flag to glob match packages.

Should we add these as options too?

@wesleytodd
Copy link

Probably not as solid an argument, but I think there's some value to keeping it close to what already exists out there.

I think there is value in this argument as long as we don't blindly follow.

I think the --scope flag seems good, but the name is confusing because npm already has a concept called scope for package names. I do like it single flag with a glob pattern approach.

The Yarn versionis much less optimal IMO. Not only should we try to avoid having two ways to do the same thing, I think the positional arguments version is harder to understand for the same reasons positional options in a function are hard, you have to memorize the order. Obviously in this case it is rather simple, but it is a philosophical thing which I find worth sticking to in most cases.

@isaacs
Copy link
Contributor

isaacs commented Jul 15, 2020

My vote is npm workspace foo <cmd>. Add ws as an alias for workspace. Abbreviations will ensure that w works just as well. Canonical command name is "workspace".

That's very easy to implement, doesn't create a "flags are config values" footgun (which --scope kind of does, though that's a more fuzzy distinction in that case).

Bottom line is whether npm ws foo test is a fundamentally different command from npm test, or if it's npm test being run in a different way.

My view is that it's a whole different kind of thing, so it'd be surprising and weird (to me, at least) to do npm config set workspace foo or npm_config_workspace=foo npm test.

But if you see "what workspace am I in" as something akin to doing npm test --prefix=./workspaces/foo (which, to be fair, is similar), then it should be a config value. In that case, we should not have a separate command for this purpose (though we might for, say, adding a new workspace to the project, creating workspace aliases, etc.) If we go that route, then the appropriate answer is --workspace as the canonical config value, with -w and -ws as aliases to it.

Questions that might help drive to the heart of this distinction:

  • Should the script being run with npm test have a npm_config_workspace=foo in the environment? (If so, probably a config. If not, probably a separate command.)
  • Would you ever want to specify a given workspace in the project-level .npmrc file for any reason? (If so, probably a config. If not probably a separate command.)
  • Would you expect npm workspace add test to (a) run the test command in the workspace named add? Or (b) add a new workspace named test? If (a), then likely a separate command. If (b), then we ought to not use the workspace command for running scripts in workspaces, and instead reserve it as a separate command for other purposes.
  • Would it be more ergonomic to (a) always have the workspace name before the command name, or (b) would you want to sometimes define it afterwards? If (a), likely a separate command. If (b), almost certainly a config value.
  • Would a workspace script ever need to run scripts from one of its sibling workspaces? Ie, the running tests in the app workspace kicks off tests in the core workspace as well. If so, this will be challenging as a config value (because it'll have to override the --workspace config from the environment).

I think I might've just talked myself into using npm test --workspace=foo rather than npm workspace foo test, but we can drive it a bit further in the call shortly.

@ljharb
Copy link
Contributor

ljharb commented Jul 15, 2020

Should the script being run with npm test have a npm_config_workspace=foo in the environment?

It would not make any sense to me for a script to be able to run differently based on whether it was ran from the top-level, or ran in situ, so on this one i'd land on "not + separate command"

Would you ever want to specify a given workspace in the project-level .npmrc file for any reason?

no, but i might want to do it via env vars.

Would you expect npm workspace add test to

Strongly B, although I could get used to A.

Would it be more ergonomic to

Either one seems fine to me.

Would a workspace script ever need to run scripts from one of its sibling workspaces?

No, that sounds like a violation of encapsulation and separation of concerns; siblings should not know about each other (except as linked dependencies, of course).


On balance I think that leaves me leaning towards npm test --workspace=foo, i guess?

@isaacs
Copy link
Contributor

isaacs commented Jul 16, 2020

From discussion:

  • don't ever put npm_config_workspace in the run-script environment (assuming we land on "config" vs "command" for this)

@ruyadorno
Copy link
Contributor Author

I'll make sure to document these results in the RFC: #117

@darcyclarke darcyclarke removed the Agenda will be discussed at the Open RFC call label Jul 22, 2020
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

No branches or pull requests

6 participants