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

Reconsider the exports definition #2968

Closed
dominykas opened this issue Sep 18, 2023 · 5 comments · Fixed by #2980
Closed

Reconsider the exports definition #2968

dominykas opened this issue Sep 18, 2023 · 5 comments · Fixed by #2980
Labels

Comments

@dominykas
Copy link
Contributor

New feature motivation

72ab317 defined the exports in the package.json, which makes deep imports no longer possible.

While I realize that these are for internal usage, and are not covered by semver, they are nevertheless useful (to be used at own risk, of course).

My use case: functionality, which uses the getCommits from semantic-release/lib/git.js and semantic-release/lib/get-config.js to call plugins.analyzeCommits so that a matching label (bug, enhancement, etc) can be added on a PR.

But even so, I'd love for semantic-release to be usable as a library, not just as a CLI.

New feature description

Please revert 72ab317 and keep things as they were.

New feature implementation

No response

@travi
Copy link
Member

travi commented Sep 18, 2023

thanks for raising the concern to us, @dominykas!

72ab317 defined the exports in the package.json, which makes deep imports no longer possible.

this was intended as cleanup in this release since we realized that we missed doing it with our conversion to esm. we were mostly taking this step now since we were releasing other breaking changes. our intent was not to limit valid use cases.

i would prefer to avoid completely reverting the exports definition, but i'm hopeful we could find a middle ground.

I'd love for semantic-release to be usable as a library, not just as a CLI.

we are exporting the index to support direct execution of the full process outside of direct cli usage since we are aware that is useful in some situations. i understand that doesnt cover everything, so i'm interested to understand further

While I realize that these are for internal usage, and are not covered by semver, they are nevertheless useful (to be used at own risk, of course).

the reality is that we arent aware of all of the ways that folks have found to make use of the "private" api, so we made this change under the assumption that it wouldnt be too restrictive. we arent completely against exposing more, but want to do so more intentionally for known use cases so that we can account for them and prevent breaking again in the future.

My use case: functionality, which uses the getCommits from semantic-release/lib/git.js and semantic-release/lib/get-config.js to call plugins.analyzeCommits so that a matching label (bug, enhancement, etc) can be added on a PR.

do you happen to have a public example of such usage? the more we understand the usecase, the better we can consider how to expose what might be needed. if you have a proposed alternative exports map definition that would expose the pieces that you need for your use case, that could also help us understand and consider an alternative.

longer term, we intend to decompose the core package further, hopefully to the point where the programatic api would be a separate package and the cli package would mostly just wrap that api. we obviously arent to that point yet, but the more clearly we can define needs like this, the more informed we can be when we approach that effort.

in the short term, can you continue using the previous version until we work through an appropriate way to solve this?

theoludwig added a commit to theoludwig/theoludwig that referenced this issue Sep 18, 2023
@dominykas
Copy link
Contributor Author

dominykas commented Sep 19, 2023

Thanks for considering this!

in the short term, can you continue using the previous version until we work through an appropriate way to solve this?

Yes, that's the plan for now.

I'm happy to contribute any fixes we agree on, if that helps.

if you have a proposed alternative exports map definition that would expose the pieces that you need for your use case, that could also help us understand and consider an alternative.

Here's the current import we're doing:

import { getCommits } from 'semantic-release/lib/git.js';
import GetConfig from 'semantic-release/lib/get-config.js';
import GetLogger from 'semantic-release/lib/get-logger.js';

I haven't looked into how this converts into an exports map just yet, but lets take a look at the usage below...

do you happen to have a public example of such usage?

This is the relevant part of the code that we have (hope I didn't mess up the copy/pasting):

  const context = {
    cwd: process.cwd(),
    env: process.env,
    logger: GetLogger({ // <-- logger required on the context
      stderr: process.stderr,
      stdout: process.env.DEBUG ? process.stdout : DevNull(),
    }),
    branch: {
      name: 'master', // <-- force the `master` branch config to be used - we are on a PR, but want to know what will happen upon merging
      type: 'release',
    },
  };
  const opts = {
    extends: getReleaseConfigPath(), // <-- points to our shared global release config
  };

  const { plugins, options } = await GetConfig(context, opts);  // <-- constructs the remaining bits of the `context`
  context.options = options;

  /* snip */

  context.commits = await getCommits(base.sha, head.sha, {});

  const level = await plugins.analyzeCommits(context);

  /* pseudo code bellow */
  const label = levelToLabel(level)
  addLabelToPr(label);

So, to summarize the above - we initialize context, with almost zero customizations - it would be real nice to have that done as a single step, of course, but this would need to be exported for sure.

And then we getCommits for the delta of the PR - this is a library function and is easy to reimplement ourselves if need be, but the whole git.js has very useful utilities which could be handy as a separate package or an export.

And we then call analyzeCommits to determine the bump level (which we then convert into a label on a PR).

We could probably drive this through the central semantic-release function with the dryRun: true (we do this in some other cases - having it exposed as a function is really handy! thanks!), however that's probably going to be slower, plus we're running it from a PR branch, but force the master branch config to basically "lets pretend it's master so that we can determine what would happen if we merge this" - not sure if we have that option out of the box.

The use case here is probably closely related to #753.

@prisis
Copy link

prisis commented Sep 21, 2023

Without the export for

  • "semantic-release/lib/get-config.js"
  • "semantic-release/lib/branches/get-tags.js"
  • "semantic-release/package.json"

the https://github.com/qiwi/multi-semantic-release cant be upgraded to the latest version

@travi
Copy link
Member

travi commented Sep 22, 2023

debated between widening exports to and exports map and fully reverting the exports definition. ended up deciding to revert for now to spend more intentional effort on defining a programatic api that could have more chance of lasting without ending up just exposing all of the internal details anyway.

once #2980 is merged, use cases that were blocked as a result of this change should work again. we will be asking for more information in #2978 related to our path forward, so we hope folks will be willing to help us shape that. i can't promise that all use cases will be enabled how they are today, but we do want to encourage extension of semantic-release is ways that the official packages may not officially support, so we hope to work through available options to enable the use cases that folks have found ways to enable with the existing implementation.

@github-actions
Copy link

🎉 This issue has been resolved in version 22.0.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants