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

added tryGetVersion method #69

Merged
merged 2 commits into from
May 5, 2022
Merged

added tryGetVersion method #69

merged 2 commits into from
May 5, 2022

Conversation

Meowhal
Copy link
Owner

@Meowhal Meowhal commented May 5, 2022

fix #68
When launched from node, the version is obtained by reading package.json, and if package.json is not found, the version is displayed as 0.0.0.

Copy link
Contributor

@xayanide xayanide left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there should be a way of using 1 return statement on the tryGetVersion method though. I have thought of an optional alternative.

  private tryGetVersion(): string {
    let version = `${process.env.npm_package_version}`;
    if (!process.env.npm_package_version) {
      try {
        const packag = require("../package.json");
        version = packag.version;
      } catch {
        version = "0.0.0";
      }
    }
    // it is possible for package.json to exist but with no version field, so it can still return as undefined
    return version;
  }

The way it is implemented right now looks good to me, also yours doesn't waste any more resources if process.env.npm_package_version is defined which is nice.


Another implementation without using process.env.npm_package_version

This would return the version as 0.0.0 if

  • Package.json file do not exist
  • version field do not exist in the file.

Running !info or !version with this, version will be defined (0.0.0 if falsy) no matter if the process was run with npm script or node with this one. I hope.

  private tryGetVersion(): string {
    try {
      const packag = require("../package.json");
      // "version": "1.5.17" field missing
      if (!packag.version) return "0.0.0";
      return packag.version;
    } catch {
      // handle module not found error/crash
      return "0.0.0";
    }
  }

@Meowhal
Copy link
Owner Author

Meowhal commented May 5, 2022

I looked at your pre-edited code and realized that it is possible the version does not exist in package.json. I will fix the code later.

@Meowhal Meowhal merged commit 5e14681 into dev May 5, 2022
@Meowhal
Copy link
Owner Author

Meowhal commented May 5, 2022

Finally, I commited this code. Your insights were very helpful.

private tryGetVersion(): string {
  if (process.env.npm_package_version) return process.env.npm_package_version;
  try {
    return require("../package.json").version ?? "0.0.0";
  } catch {
    return "0.0.0";
  }
}

I prefer the early return style. But i'm not going to debate which style is better. What is more important with such a small code is whether we are achieving our objectives.😉

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 this pull request may close these issues.

None yet

2 participants