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

Excluding default from conditions #30

Open
brettz9 opened this issue Jun 6, 2023 · 8 comments · May be fixed by #31
Open

Excluding default from conditions #30

brettz9 opened this issue Jun 6, 2023 · 8 comments · May be fixed by #31

Comments

@brettz9
Copy link

brettz9 commented Jun 6, 2023

Hi,

Great to find such a library, thank you.

I was wondering whether you might add an option such as noDefault to remove checking for default in exports.

https://github.com/lukeed/resolve.exports/blob/master/src/utils.ts#L16

I'm only interested in types or typings.

Thanks!

@regseb
Copy link

regseb commented Jun 15, 2023

A proposal with option.noDefault:

Type: boolean
Default: false

When truthy, the "default" field is removed from the list of allowed/known conditions.

resolve.exports(pkg, '.', { noDefault: true });
//=> Conditions: ["import", "node"]

resolve.exports(pkg, '.', {
  noDefault: true,
  unsafe: true,
  conditions: ['types'],
});
//=> Conditions: ["types"]
export function conditions(options: t.Options): Set<t.Condition> {
  let out = new Set(options.conditions || []);
  options.noDefault || out.add('default');
  options.unsafe || out.add(options.require ? 'require' : 'import');
  options.unsafe || out.add(options.browser ? 'browser' : 'node');
  return out;
}

An other proposal with option.types:

Type: boolean
Default: false

When truthy, the "types" field is set to the list of allowed/known conditions.

resolve.exports(pkg, '.', { types: true });
//=> Conditions: ["types"]

resolve.exports(pkg, '.', {
  types: true,
  conditions: ['typings'],
});
//=> Conditions: ["types", "typings"]
export function conditions(options: t.Options): Set<t.Condition> {
  let out = new Set(options.conditions || []);
  if (options.unsafe) {
    out.add('default');
  } else if (options.types) {
    out.add('types');
  } else {
    out.add('default');
    out.add(options.require ? 'require' : 'import');
    out.add(options.browser ? 'browser' : 'node');
  }
  return out;
}

@brettz9
Copy link
Author

brettz9 commented Jun 15, 2023

The first option sounds the most reusable (in case there were any other use cases for dropping default) though either should work for us.

@ljharb
Copy link

ljharb commented Jun 15, 2023

Why not simply an array of the conditions you want to check? Then it's not specific to "default", or to types.

@brettz9
Copy link
Author

brettz9 commented Jun 15, 2023

Why not simply an array of the conditions you want to check? Then it's not specific to "default", or to types.

That's what conditions already is, but because the existing behavior is to also add default, therein lies the problem. If you're suggesting changing it to avoid the auto-adding of default, then of course, that'd be a breaking change.

@ljharb
Copy link

ljharb commented Jun 15, 2023

ahhhh, gotcha. then yeah i'd go with noDefault, and then in the next semver-major, remove that option and also the auto-adding behavior.

@brettz9
Copy link
Author

brettz9 commented Jun 15, 2023

that'd work for me too, though I do see an argument for having default preferred by default.

@regseb
Copy link

regseb commented Jun 16, 2023

A third proposal with negative conditions:

If you want to undo one of default conditions, you can use the ! prefix, for example: ['!node'].

resolve.exports(pkg, '.');
//=> Conditions: ["default", "import", "node"]

resolve.exports(pkg, '.', { conditions: ['!import'] });
//=> Conditions: ["default", "node"]

resolve.exports(pkg, '.', { conditions: ['!node', 'deno'] });
//=> Conditions: ["default", "import", "deno"]

resolve.exports(pkg, '.', { conditions: ['!default', '!import', '!node', 'types'] });
//=> Conditions: ["types"]

resolve.exports(pkg, '.', { conditions: ['!import', 'require'] }); // Same as { require: true }.
//=> Conditions: ["default", "require", "node"]

resolve.exports(pkg, '.', { conditions: ['!node', 'browser'] }); // Same as { browser: true }.
//=> Conditions: ["default", "import", "browser"]

resolve.exports(pkg, '.', { conditions: ['!import', '!node'] }); // Same as { unsafe: true }.
//=> Conditions: ["default"]
export function conditions(options: t.Options): Set<t.Condition> {
  let out = new Set(['default']);
  options.unsafe || out.add(options.require ? 'require' : 'import');
  options.unsafe || out.add(options.browser ? 'browser' : 'node');
  for (const condition of options.conditions || []) {
    if (condition.startsWith('!')) {
      out.delete(condition.slice(1));
    } else {
      out.add(condition);
    }
  }
  return out;
}

If you want to remove options.require, options.browser and options.unsafe:

export function conditions(options: t.Options): Set<t.Condition> {
  let out = new Set(['default', 'import', 'node']);
  for (const condition of options.conditions || []) {
    if (condition.startsWith('!')) {
      out.delete(condition.slice(1));
    } else {
      out.add(condition);
    }
  }
  return out;
}

@regseb regseb linked a pull request Jul 27, 2023 that will close this issue
@brettz9
Copy link
Author

brettz9 commented Aug 4, 2023

I'm fine myself with the latter approach added to #31 . If that's ok, can it be reviewed?

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

Successfully merging a pull request may close this issue.

3 participants