RFC for `context` config parameter #336

nlf · 2021-03-08T19:57:40Z

This is a suggestion to allow for users to leverage the npm CLI to modify their project-level .npmrc files, as well as to remove ambiguity and inconsistency in the behavior of npm exec

nlf · 2021-03-08T23:13:41Z

~~this is going to be expanded to apply the where parameter to npm exec tomorrow~~ updated!

ruyadorno · 2021-03-09T01:45:21Z

heads up: the name "where" might be a little confusing in some situations and problematic in some other ones

an example of possible confusion are some places internally to the cli that uses where as the cwd value, there are quite a lot of usage associating where with that meaning:
an example in which it could become actually problematic is the fact that pacote (and probably more of our ecosystem of packages) expects a where property as the cwd value - if we were to add where to flatOptions it can quickly cause a lot of trouble.

(spoiler alert: I have all that info on top of my mind since I was thinking about turning this repeating where logic into its own mixin of sorts for all commands 😅 )

Given all that context, may I suggest picking a different name to err on the safe side?

nlf · 2021-03-09T02:29:30Z

fair enough, how about location? I'll try to think of others.. where felt so right

ruyadorno · 2021-03-09T14:58:28Z

maybe it's the other way round and we might want to migrate the current usage of where to something else... I particularly prefer using cwd instead (and that's also used in some share of our ecosystem of packages) - Given the current timing with the multiple efforts on cleaning up configs, that might be a good alternative to propose too 👍

nlf · 2021-03-09T15:08:55Z

i like that better, where is such a perfect fit for this.

i'll use your head start and see if i can figure out what other modules in our ecosystem care about the where option so we can have a comprehensive list here of what will need modified.

nlf · 2021-03-09T16:09:49Z

i didn't do the work ahead of time to audit the dependencies, but did make a note under implementation that work must be done before implementation here

nlf · 2021-06-16T22:13:14Z

due to the ambiguity of where as a config parameter, i've renamed it to context in the spirit of tools like kubectl which use context to refer to a specific portion of configuration.

our implementation will differ, however, as instead of referring to keys within one config file like kubectl the npm config command will use the context to refer to a named configuration location without the need to specify paths. for npm exec the context will refer to a named source a binary should be found in.

we chose this over something more specific like --config-location or --bin-location to avoid the implication that a directory can be provided. the context will always specifically be an enum with known values. language like "run foo from the global context" or "set the config in the local context" make sense, and there appears to be extremely minimal overlap with existing options in our dependencies.

darcyclarke · 2021-06-16T22:19:47Z

I'm bias but think this is the right approach/nomenclature & helps standardize/centralize our understanding of these existing/known contexts

darcyclarke · 2021-06-17T17:04:56Z

accepted/0034-context-config-parameter.md

@@ -0,0 +1,116 @@
+# {{TITLE: Add a `context` parameter for `npm config` and `npm exec` commands}}


@nlf I think we can remove the {{TITLE: ... }} wrapping syntax

darcyclarke

Lets try to remove/update some of bikeshedding/unresolved questions

darcyclarke · 2021-06-17T17:07:05Z

accepted/0034-context-config-parameter.md

+
+## Unresolved Questions and Bikeshedding
+
+~Is `where` the best name for this config? What if there are too many conflicts in dependencies?~ EDIT: Document updated to use `context` instead of `where`.


I think we can remove this question since we've updated the name & consider that to be better/more concise then where was.

Sorry if I missed this discussion - I do not find "context" to be clear at all. It's a word with tons of conflated meanings.

you didn't miss it, we did some bike shedding trying to come up with something else outside of the rfc calls. we also discussed having separate flags for each purpose like --config-location/--bin-location but there was some concern that "location" implies the value can be a path, which is not something we want to support. most alternatives i came up with had the same problem, such as --config-source. something like --config-type/--bin-type could work maybe?

we went with context because it is somewhat vague, but is language that can be reasoned about. if you've got better suggestions, though, i'm all ears. it's time to pick a color for our bike shed so we can get to painting it.

It is a location tho - location doesn’t necessarily imply path, and since it’s an enum, it’d immediately error if an invalid value was passed, so any confusion would be rapidly dispelled.

what about just --location? npm config set foo=bar --location=user isn't terrible, nor is npx --location=remote tap, -L is open as a short form too (-l is --long)

That seems fine with me.

darcyclarke · 2021-06-17T17:10:02Z

accepted/0034-context-config-parameter.md

+
+~Is `where` the best name for this config? What if there are too many conflicts in dependencies?~ EDIT: Document updated to use `context` instead of `where`.
+
+What would the shortcut be? `-c` is currently the short form of `--call`.


-c is --call & -C is --prefix (a bit confusing there on that second one for sure....) so I feel like we could use -x

darcyclarke · 2021-06-17T18:16:57Z

accepted/0034-context-config-parameter.md

+
+### `npm config`
+
+Should we prevent a user from setting protected fields like `_auth` or `_token` to a local config?


Let's stay consistent with whatever is supported by npm config by default &/or when run in the global context. If we support different behaviour in the user or global contexts today I'd wonder why; The whole point of unlocking local/project-level context is to eliminate the need to manually manage an .npmrc file. Limiting the fields you can set would also reduce the usability of defining the local context

that's a fair point 👍

darcyclarke · 2021-06-17T18:22:53Z

accepted/0034-context-config-parameter.md

+
+### `npm exec`
+
+Should we prompt when multiple matches are found? If we would prompt but are unable to, what should we do?


Not sure what this means? I feel like this is already answered by the note, further up, outlining that we will follow the existing binary resolution pattern in npm exec (ref. https://github.com/npm/rfcs/pull/336/files#diff-e69970007bc5185a814912b41199ac9648ecfc70be343865a55d558f4c417028R56-R57)

iirc there was some concern about which bin was linked being non-deterministic, but after a quick read through of bin-links it seems like that should only be possible if the user passed --force (or somehow we ended up reifying something deeper in the dep tree before something shallower, which shouldn't be possible since arborist sorts deps so shallowest are first) so maybe it's not something to worry about

ljharb · 2021-09-13T21:56:36Z

@nlf should the title of this PR be updated?

nlf marked this pull request as draft March 8, 2021 23:13

nlf marked this pull request as ready for review March 9, 2021 16:03

nlf changed the title ~~chore: add rfc for where config parameter~~ RFC for where config parameter Mar 9, 2021

darcyclarke mentioned this pull request Mar 10, 2021

ℹ️ Cancelling Open RFC Call for Wednesday, March 10th, 2021 #338

Closed

darcyclarke added the Proposal label Mar 10, 2021

nlf mentioned this pull request Mar 10, 2021

rfc for improving command suggestions #339

Merged

darcyclarke added the Agenda will be discussed at the Open RFC call label Mar 17, 2021

This was referenced Mar 17, 2021

Open RFC Meeting - Wednesday, March 17, 2021, 2:00 PM EST #342

Closed

Open RFC Meeting - Wednesday, March 24, 2021, 2:00 PM EST #346

Closed

darcyclarke mentioned this pull request Apr 7, 2021

Open RFC Meeting - Wednesday, April 7, 2021, 2:00 PM EST #362

Closed

darcyclarke mentioned this pull request Apr 14, 2021

Open RFC Meeting - Wednesday, April 14, 2021, 2:00 PM EST #367

Closed

darcyclarke mentioned this pull request May 5, 2021

Open RFC Meeting - Wednesday, May 5, 2021, 2:00 PM EST #374

Closed

This was referenced May 12, 2021

Open RFC Meeting - Wednesday, May 12, 2021, 2:00 PM EST #381

Closed

🚫 Cancelled: Open RFC Meeting - Wednesday, May 19, 2021 #383

Closed

This was referenced Jun 9, 2021

Open RFC Meeting - Wednesday, June 9, 2021, 2:00 PM EST #396

Closed

Open RFC Meeting - Wednesday, June 16, 2021, 2:00 PM EST #399

Closed

nlf changed the title ~~RFC for where config parameter~~ RFC for context config parameter Jun 16, 2021

darcyclarke removed the Agenda will be discussed at the Open RFC call label Jun 17, 2021

darcyclarke reviewed Jun 17, 2021

View reviewed changes

darcyclarke suggested changes Jun 17, 2021

View reviewed changes

nlf added 2 commits September 13, 2021 14:05

chore: add rfc for where config parameter

2687845

expand rfc to cover usage for npm exec

de7c6b7

nlf added 3 commits September 13, 2021 14:05

add note about shortcut

4233d3c

change config name to context, change project value to local

6dd3713

chore: update context to location

0a63ed2

nlf force-pushed the nlf/where-option branch from 08b45ae to 0a63ed2 Compare September 13, 2021 21:09

chore: update questions

8a1437d

nlf merged commit 31a691b into latest Sep 13, 2021

nlf deleted the nlf/where-option branch September 13, 2021 21:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

RFC for `context` config parameter #336

RFC for `context` config parameter #336

nlf commented Mar 8, 2021 •

edited

Loading

nlf commented Mar 8, 2021 •

edited

Loading

ruyadorno commented Mar 9, 2021

nlf commented Mar 9, 2021

ruyadorno commented Mar 9, 2021 •

edited

Loading

nlf commented Mar 9, 2021

nlf commented Mar 9, 2021

nlf commented Jun 16, 2021

darcyclarke commented Jun 16, 2021 •

edited

Loading

darcyclarke Jun 17, 2021

darcyclarke left a comment

darcyclarke Jun 17, 2021

ljharb Jun 17, 2021

nlf Jun 19, 2021

ljharb Jun 19, 2021

nlf Jun 21, 2021

ljharb Jun 21, 2021

darcyclarke Jun 17, 2021

darcyclarke Jun 17, 2021

nlf Jun 21, 2021

darcyclarke Jun 17, 2021

nlf Jun 21, 2021

ljharb commented Sep 13, 2021

		@@ -0,0 +1,116 @@
		# {{TITLE: Add a `context` parameter for `npm config` and `npm exec` commands}}


		## Unresolved Questions and Bikeshedding

		~Is `where` the best name for this config? What if there are too many conflicts in dependencies?~ EDIT: Document updated to use `context` instead of `where`.


		~Is `where` the best name for this config? What if there are too many conflicts in dependencies?~ EDIT: Document updated to use `context` instead of `where`.

		What would the shortcut be? `-c` is currently the short form of `--call`.


		### `npm config`

		Should we prevent a user from setting protected fields like `_auth` or `_token` to a local config?


		### `npm exec`

		Should we prompt when multiple matches are found? If we would prompt but are unable to, what should we do?

RFC for context config parameter #336

RFC for context config parameter #336

Conversation

nlf commented Mar 8, 2021 • edited Loading

nlf commented Mar 8, 2021 • edited Loading

ruyadorno commented Mar 9, 2021

nlf commented Mar 9, 2021

ruyadorno commented Mar 9, 2021 • edited Loading

nlf commented Mar 9, 2021

nlf commented Mar 9, 2021

nlf commented Jun 16, 2021

darcyclarke commented Jun 16, 2021 • edited Loading

Choose a reason for hiding this comment

darcyclarke left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ljharb commented Sep 13, 2021

RFC for `context` config parameter #336

RFC for `context` config parameter #336

nlf commented Mar 8, 2021 •

edited

Loading

nlf commented Mar 8, 2021 •

edited

Loading

ruyadorno commented Mar 9, 2021 •

edited

Loading

darcyclarke commented Jun 16, 2021 •

edited

Loading