Upgrade command: Keep package.json version prefix

[lcnvdl]

When you create a new FoalTS project, all @foal dependencies begins with the '^' prefix. I think maybe it's a good idea to keep it.

See the NPM docs and semver docs:

~version “Approximately equivalent to version”, will update you to all future patch versions, without incrementing the minor version. ~1.2.3 will use releases from 1.2.3 to <1.3.0.

^version “Compatible with version”, will update you to all future minor/patch versions, without incrementing the major version. ^1.2.3 will use releases from 1.2.3 to <2.0.0.

Issue

Version prefix is lost after upgrading Foal with the command foal upgrade.

Solution and steps

Checklist

• Add/update/check docs (code comments and docs/ folder).
• Add/update/check tests.
• Update/check the cli generators.

[LoicPoullain]

That’s a nice feature! Every time I ran this command, I wished it had kept the prefix! 😁

A few comments on the PR:

• Depending on the package’s dependencies, the setOrUpdateProjectDependency method may create or update the dependency. In the case of a creation, the name keepExistingPrefix might surprise the reader, as no prefix actually exists. Could it be interesting to improve the name so that it better reflects this? Some ideas: keepExistingPrefixIfAny, keepExistingPrefixIfExists, keepExistingPrefixOnUpdate, keepPrefixIfAny, keepPrefixIfExists, keepPrefixOnUpdate.

• Apart from the upgrade command, the other CLI commands don't need to keep the prefix of an existing dependency or do anything specific about prefixes. Would it be worth updating the setOrUpdateProjectDependency method interface so that only the { keepExistingPrefix: boolean = false } option is added? This way, the rest of the CLI code doesn't need to be updated and more straightforward statements like fs.setOrUpdateProjectDependencyOnlyIf(yaml, 'yamljs', '~0.3.0') in create-app still work. The prefix would then still be specified with the version number.

• To handle the prefix replacement, I wonder if we can do this directly in the FileSystem service with more reduced code. For example, the line pkg.devDependencies[name] = version; could be replaced by something like this:

  function getPrefix(version: string): string {
    const firstChar = version[0];
    return ['^', '~'].includes(firstChar) ? firstChar : '';
  }
  
  function replacePrefix(version: string, prefix: string): string {
    const versionWithoutPrefix = version.startsWith(['^', '~']) ? version.substring(1) : version;
    return `${prefix}${version}`;
  }
  
  if (pkg.devDependencies[name] && keepExistingPrefix) {
    const existingPrefix = getPrefix(pkg.devDependencies[name]);
    pkg.devDependencies[name] = replacePrefix(version, existingPrefix);
  } else {
    pkg.devDependencies[name] = version;
  }

  In this way, a layer of abstraction would be removed and the reader would enter the function's behavior more directly, with one click. It would also reduce the amount of code to be maintained in the future.

What do you think?

[lcnvdl]

That’s a nice feature! Every time I ran this command, I wished it had kept the prefix! 😁

A few comments on the PR:

• Depending on the package’s dependencies, the setOrUpdateProjectDependency method may create or update the dependency. In the case of a creation, the name keepExistingPrefix might surprise the reader, as no prefix actually exists. Could it be interesting to improve the name so that it better reflects this? Some ideas: keepExistingPrefixIfAny, keepExistingPrefixIfExists, keepExistingPrefixOnUpdate, keepPrefixIfAny, keepPrefixIfExists, keepPrefixOnUpdate.

I agree with you! Thanks for the feedback!

• Apart from the upgrade command, the other CLI commands don't need to keep the prefix of an existing dependency or do anything specific about prefixes. Would it be worth updating the setOrUpdateProjectDependency method interface so that only the { keepExistingPrefix: boolean = false } option is added? This way, the rest of the CLI code doesn't need to be updated and more straightforward statements like fs.setOrUpdateProjectDependencyOnlyIf(yaml, 'yamljs', '~0.3.0') in create-app still work. The prefix would then still be specified with the version number.

That's a good Idea! I'll do it

• To handle the prefix replacement, I wonder if we can do this directly in the FileSystem service with more reduced code. For example, the line pkg.devDependencies[name] = version; could be replaced by something like this:

  function getPrefix(version: string): string {
    const firstChar = version[0];
    return ['^', '~'].includes(firstChar) ? firstChar : '';
  }

Super simple, I like it!

function replacePrefix(version: string, prefix: string): string {
const versionWithoutPrefix = version.startsWith(['^', '~']) ? version.substring(1) : version;
return ${prefix}${version};
}

startsWith doesn't work with an array as param, but I understand your idea.

if (pkg.devDependencies[name] && keepExistingPrefix) {
const existingPrefix = getPrefix(pkg.devDependencies[name]);
pkg.devDependencies[name] = replacePrefix(version, existingPrefix);
} else {
pkg.devDependencies[name] = version;
}

    
      
    

      
    

    
  
In this way, a layer of abstraction would be removed and the reader would enter the function's behavior more directly, with one click. It would also reduce the amount of code to be maintained in the future.

What do you think?

I have to review the code, but I think this leads to code duplication. That's why I've isolated some funcionalities in a different object. But I understand that your priority is to use less amount of files/classes, accepting the cost of having duplicated code blocks.

If that's the case, then I can do a refactor, using your suggestion.