jiti icon indicating copy to clipboard operation
jiti copied to clipboard

feat: support custom user conditions

Open ernestostifano opened this issue 7 months ago • 6 comments

Resolves: https://github.com/unjs/jiti/issues/383, https://github.com/unjs/jiti/issues/369

Main Changes:

  • Added support for custom user conditions:
    • Via JitiOptions and JitiResolveOptions, so, consistently across all programmatic APIs.
    • Via JITI_CONDITIONS environment variable.
    • Via package.json's conditions field.
  • Added support for synchronous hooks.

Added Capabilities Examples:

const jiti = createJiti(import.meta.url, {conditions: true});
const jiti = createJiti(import.meta.url, {conditions: ['development']});
const jiti = createJiti(import.meta.url, {
    conditions: {'@scope/**/*': ['development']}
});
const jiti = createJiti(import.meta.url, {
    conditions: [
        {
            match: ['@scope/**/*'],
            ignore: ['@scope/some-specific-package'],
            values: ['development']
        }
    ]
});
  • There is full minimatch support.
  • If conditions is set to true, configuration will be read from the nearest package.json, relative to cwd.
  • If conditions is set to false, or no configuration is found, then everything should behave like this PR never existed.

Details:

  • Created utils.mjs to group common code for both synchronous and asynchronous hooks.
  • Replaced exists with access according to most recent recommended approach.
  • Enabled pnpm workspaces for testing with packages.
  • Added tests for custom user conditions implementation.
  • Updated documentation.

Notes:

  • There should not be any breaking changes.
  • There should not be any relevant performance changes.
  • Made conditions -> true by default according to this Node.JS Proposal (see point 5 at "The Problem - TL;DR").
  • I noticed some differences between outputs from jiti and native methods:
    • import() !== jiti.import().
    • createRequire() -> require() !== jiti.import().
    • Static import (when using --import jiti/register) !== jiti.import().
  • The above happens without this PR and I would be happy to investigate and fix it, if needed, after merging this PR.

ernestostifano avatar Jun 13 '25 23:06 ernestostifano

@pi0 happy to make any changes if needed.

ernestostifano avatar Jun 13 '25 23:06 ernestostifano

@pi0 friendly ping. I'd love to receive some feedback. Maybe I could help maintaining this package in the future.

If you think this cannot be merged right now I could publish a forked version.

(If you prefer we review it together, I can schedule a meeting)

ernestostifano avatar Jun 23 '25 21:06 ernestostifano

This seems a good idea!

AnielloFalcone avatar Jul 01 '25 09:07 AnielloFalcone

Shamelessly pinging @pi0 - I'd love to see this merged!

pigulla avatar Aug 09 '25 11:08 pigulla

It is a risky changest with possible regressions and lots of changes.

Ideally if someone can help making another PR with minimum enough (few lines of diff) to add custom resolve conditions it can speedup adding this feature.

pi0 avatar Aug 10 '25 16:08 pi0

@pi0 I understand your concerns, but all these features are tightly related and I think they all add value considering how Node.Js is evolving.

I tried to separate changes logically into multiple commits, all automatic tests are passing and I added new ones covering the new changes.

I also performed many manual tests installing the library locally.

I have a good level of confidence that no regressions were introduced.

If you want, I would be available to review together.

ernestostifano avatar Nov 19 '25 13:11 ernestostifano