module: custom --conditions flag option #34637
Conversation
nodejs-github-bot
added
c++
|
we likely should ban known contextual conditions |
Co-authored-by: Jordan Harband <[email protected]> Co-authored-by: Jan Olaf Krems <[email protected]>
I was thinking about this case and the worst case seems to be having CJS resolve ESM or ESM resolve the CJS path depending on how the ordering works out. Neither of these seem that bad to me though if users want to do it? |
|
@guybedford I agree it isn't fatal in any way if users want to set those for w/e reason, i just have concerns since we have statements in the docs about being exclusive being invalidated. |
|
@bmeck yes worth thinking about, although the main property behind the exclusive definition was that one or the other will always be matched (so there are no fallbacks when both are set in any environment), but that property is retained. |
|
Given that ESM and CJS are ambiguous, it seems like a very not good idea to allow accidental mis-parsing. |
The conditions do not affect the static format of a file and even if you set the |
|
@bmeck so to confirm, you're saying even if i have a file that contains nothing but |
|
@ljharb not via this PR, I might be able to fiddle with the old CJS loader to get it to parse wrong with effort; but to my knowledge it is never possible with the ESM loader. |
I'm pretty sure this should be impossible, at least for accidental cases, since we made |
|
@GeoffreyBooth agreed |
PR-URL: #34637 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Jan Krems <[email protected]>
|
Landed in 77a515c. |
PR-URL: #34637 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Jan Krems <[email protected]>
PR-URL: #34637 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Jan Krems <[email protected]>
Old versions of mocha break after nodejs#34637. This was a bug in mocha, but since this is a widely used module we can expect ecosystem breakage until modules are updated to the latest version of mocha. Drop the conflicting `-u` alias -- we can potentially bring it back once modules have been updated.
Old versions of mocha break after #34637. This was a bug in mocha, but since this is a widely used module we can expect ecosystem breakage until modules are updated to the latest version of mocha. Drop the conflicting `-u` alias -- we can potentially bring it back once modules have been updated. PR-URL: #34935 Refs: mochajs/mocha#4417 Refs: #34637 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Jan Krems <[email protected]> Reviewed-By: Beth Griggs <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Shelley Vohr <[email protected]>
Old versions of mocha break after #34637. This was a bug in mocha, but since this is a widely used module we can expect ecosystem breakage until modules are updated to the latest version of mocha. Drop the conflicting `-u` alias -- we can potentially bring it back once modules have been updated. PR-URL: #34935 Refs: mochajs/mocha#4417 Refs: #34637 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Jan Krems <[email protected]> Reviewed-By: Beth Griggs <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Shelley Vohr <[email protected]>
|
If this is backported to v12.x, it should come together with #34935 |
|
Thanks, this likely should be backported, if not for any other reason than to simplify the backporting of subsequent modules PRs. |
PR-URL: nodejs#34637 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Jan Krems <[email protected]>
Old versions of mocha break after nodejs#34637. This was a bug in mocha, but since this is a widely used module we can expect ecosystem breakage until modules are updated to the latest version of mocha. Drop the conflicting `-u` alias -- we can potentially bring it back once modules have been updated. PR-URL: nodejs#34935 Refs: mochajs/mocha#4417 Refs: nodejs#34637 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Jan Krems <[email protected]> Reviewed-By: Beth Griggs <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Shelley Vohr <[email protected]>
PR-URL: #34637 Backport-PR-URL: #35385 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Jan Krems <[email protected]>
Old versions of mocha break after #34637. This was a bug in mocha, but since this is a widely used module we can expect ecosystem breakage until modules are updated to the latest version of mocha. Drop the conflicting `-u` alias -- we can potentially bring it back once modules have been updated. PR-URL: #34935 Backport-PR-URL: #35385 Refs: mochajs/mocha#4417 Refs: #34637 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Jan Krems <[email protected]> Reviewed-By: Beth Griggs <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Shelley Vohr <[email protected]>
This implements a
node --conditions=customflag for supporting custom resolution conditions in conditional exports, as previously discussed in nodejs/modules#537. A short alias variant,-u=customis also provided, which stands for "user conditions".Any custom condition names can be provided as repeated flags, but the core
node,default,requireandimportconditions cannot be changed.In the process a couple of conditional resolution edge ase bugs are fixed as well here.
@nodejs/modules-active-members
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes