toolsplus / nx-forge

Atlassian Forge plugin for Nx
https://toolsplus.github.io/nx-forge/
MIT License
17 stars 4 forks source link

Every app generation resets package versions of @forge/api and @forge/resolver to `latest`... #118

Closed zjkipping closed 1 month ago

zjkipping commented 1 month ago

Every time we generate a new forge application using this plugin our package.json is overwritten for the @forge/api and @forge/resolver packages to be latest. This should not happen if the packages are already present.

image

The init generator doesn't check to see if the packages are present before updating the package.json. Also latest isn't a great thing to add to the package.json to begin with.

What I have done in the past with other nx plugins I have written is the below as an example. It checks to see if the package exists on the package.json; if it doesn't, then it grabs the latest version using npm view and adds it to the package.json.

function getInstalledPackageVersion(
  tree: Tree,
  pkgName: string
): string | null {
  const { dependencies, devDependencies } = readJson(tree, 'package.json');
  const version = dependencies?.[pkgName] ?? devDependencies?.[pkgName];

  return version;
}

const deps = ['@forge/resolver', '@forge/api'];
const packageVersions: Record<string, string> = {};
deps.forEach((pkg) => {
  const packageVersion = getInstalledPackageVersion(tree, pkg);

  if (!packageVersion) {
    const version = execSync(`npm view ${pkg} version`).toString().replace('\n', '');
    packageVersions[pkg] = version
  }
});

addDependenciesToPackageJson(tree, {}, packageVersions)
tbinna commented 1 month ago

Thanks for bringing this up, @zjkipping. I agree that this behavior is not ideal. I will work on a fix based on your suggestion.

tbinna commented 1 month ago

@zjkipping, I am thinking of following the implementation of the official Nx plugins by adding the two options keepExistingVersions and skipPackageJson instead (example). Both options would default to false. This behavior would then be consistent with official Nx plugins.

I still agree, though, that if keepExistingVersions is false, it should fetch the latest available package version instead of using latest.

Do you see any issue with this approach?

zjkipping commented 1 month ago

@tbinna I think this sounds good since we can set a default in our nx.json for the keepExistingVersions argument. Appreciate the fast & thorough response :)