Skip to content

Config.changelog set to false, still writes CHANGELOG.MD files after exiting prerelease #896

Closed

Description

@sdirosa

Affected Packages

As far as I can tell the logic of this is in @changesets/apply-release-plan.

Problem

We have recently switched to changesets in our monorepo (very happy with it!) but we don't want to use the changelog feature (yet).

I've set "changelog": false in our .changeset/config.json. This is the full config:

{
  "$schema": "https://unpkg.com/@changesets/config@2.1.0/schema.json",
  "changelog": false,
  "commit": false,
  "fixed": [["@objectiv/*"]],
  "linked": [],
  "access": "public",
  "baseBranch": "main",
  "updateInternalDependencies": "patch",
  "ignore": []
}

When I exit a prerelease and run yarn changeset version to apply my changesets, I also end up with changelog files being created in all the packages of the monorepo. I am not sure if this happens also without pre, I haven't tested that, as we only do prerelease flows.

Some superficial investigation

From a quick check on the source code it seems the getNewChangelogEntry function correctly checks whether changelogs have been disabled to skip some initial logic, but then proceeds to return a list of packages with changelogs anyway.

Later in the code, the bit of code that processes the list of finalisedRelease entries doesn't check the config either. Instead it validates the presence of the changelog attribute. E.g. changelog && changelog.length > 0.

Unless I am completely off (I only spent a few minutes reading the code), since finalisedRelease is partially based on the output of getNewChangelogEntry, I'd guess that most probably the changelog attribute is always present and filled with something.

It's not a big deal for us, we can run a find and delete the changelogs before committing. That said, I think quite a bit of logic runs to process changelogs and it could be entirely skipped when that setting is set to false.

Proposed solution

Would be really cool if setting changelog to false in the config.json would result in no changelogs being generated at all, just package.json updates.

A quick fix could be to either skip getNewChangelogEntry when config.changelog is false, or double check whether changelogs have been disabled when processing the list of finalisedRelease and skip the changelog logic there.

I'd be very happy to try and fix this up, or test it if necessary.

Activity

  1. Andarist commented on Aug 2, 2022

    @Andarist
    Member

    I've checked the PR that introduced schema.json and it seems that false was always a valid value there:
    https://github.com/changesets/changesets/pull/132/files#diff-d3013abc54530a5bedc30c2f340c829e5dc49923a70bdf4cc469180bf047cb38R18-R21
    Types added as part of this PR also suggest so:
    https://github.com/changesets/changesets/pull/132/files#diff-e71f2026e90f1ed0c00f8711eb855f0a8810f33dbf1491f5da9c97cdd513e5b4R64
    and even more so the getNormalisedChangelogOption function from this PR:
    https://github.com/changesets/changesets/pull/132/files#diff-2eafc75a9015704b121dff9585728616f562e1625da9a53e44a86cacb94ae619R16-R21

    The validation error message doesn't specify it as allowed though:
    https://github.com/changesets/changesets/pull/132/files#diff-2eafc75a9015704b121dff9585728616f562e1625da9a53e44a86cacb94ae619R48-R54

    The same~ message is still in the codebase now and thus it should be adjusted to make it clear that false is also a valid option.

    Since this PR only declares the new config shape etc but doesn't really implement any features I've also rechecked the code at its point in time and it seems that back then there was an updateChangelog option to control what you want:

    if (config.updateChangelog) {
    logger.log("Updating changelogs...");
    // Now update the changelogs
    const changelogPaths = await updateChangelog(releaseObj, config);
    if (config.commit) {
    for (const changelogPath of changelogPaths) {
    await git.add(changelogPath, cwd);
    }
    }
    }

    This makes me believe that the original intention to allow false was to, in fact, turn off the changelog generation.

    By digging a little bit deeper we can also see that at some point with the new config it was working as intended because the "default" getChangelogFunc was returning an empty string:

    let getChangelogFunc: ChangelogFunction = () => Promise.resolve("");

    and because it stayed used when config.changelog === false then the CHANGELOG files were never getting updated here:
    if (changelog && changelog.length > 0) {
    await updateChangelog(changelogPath, changelog, name, prettierConfig);
    touchedFiles.push(changelogPath);
    }

    So this has accidentally regressed in this PR when getChangelogFunc became getChangelogFuncs:
    https://github.com/changesets/changesets/pull/147/files#diff-81efc39f0a283b4d7521917c7d32688cc909b4c27820807c7fc5e554b9ab37c7R134-R137

    TLDR: I'm confident to classify this as a bug and I would gladly accept a PR fixing this 😉

  2. sdirosa commented on Aug 2, 2022

    @sdirosa
    Author

    Thanks for the quick reply and in-depth analysis @Andarist! It really helps to see the history of the code and where this regressed.

    By the way, the documentation also describes this intention of using false as a way of disabling changelog generation:

    Where it states:

    If it is false, no changelogs will be generated.
    

    If I can make some time I'll definitely give this a shot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions