module: fix specifier resolution option value

[himself65]

Fixes: #35095

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

[nodejs-github-bot]

Review requested:

• @nodejs/modules

[MylesBorins]

The two flags are already set up as aliases in the options parser FWIW

https://github.com/nodejs/node/blob/master/src/node_options.cc#L95-L114

https://github.com/nodejs/node/blob/master/src/node_options.cc#L366-L374

[himself65]

The two flags are already set up as aliases in the options parser FWIW

https://github.com/nodejs/node/blob/master/src/node_options.cc#L95-L114

https://github.com/nodejs/node/blob/master/src/node_options.cc#L366-L374

Yep, so I found what the actually bug is,

node/src/node_options.cc

Line 101 in ccdd1bd

experimental_specifier_resolution = es_module_specifier_resolution;

assign experimental_specifier_resolution with es_module_specifier_resolution but what if we have experimental_specifier_resolution=node instead of es_module_specifier_resolution

[addaleax]

Why is --es-module-specifier-resolution not set up as an actual alias of --experimental-specifier-resolution (or vice versa) using AddAlias()? That doesn’t seem great in the first place

[MylesBorins]

@addaleax I think I set it up this way to ensure that both aliases were not used at the same time, which may have been a bit of overengineering. We could likely simplify the logic or simply remove the alias on master / 15 tbh

[addaleax]

@MylesBorins Yeah, I think that’s something we should definitely simplify. Having it be a plain alias turns it into a single line with basically 0 maintenance overhead (which can and should be kept indefinitely according to our policies).

@himself65 Do you want to turn --experimental-specifier-resolution into an alias for --es-module-specifier-resolution yourself, or should I open a PR with that?

[MylesBorins]

@addaleax would a simple alias fail if both the original and alias are passed with different values?

[addaleax]

@MylesBorins No, but if necessary we could make that happen. However, I don’t think it’s worth all the complexity of this code.

[MylesBorins]

@addaleax SGTM.

[bmeck]

Do we have a documented priority of the option value if 2 conflicting values are supplied?

[addaleax]

Do we have a documented priority of the option value if 2 conflicting values are supplied?

The last specified value is used. This is just like when an option that has a single value is used twice.

[bmeck]

@addaleax any behavior seems fine, just want docs.

[addaleax]

Thank you :)

[MylesBorins]

LGTM

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/33124/

[Trott]

Landed in 20a074b0cd55

Landed in 22c52aa