Typescript Support

[JefferMarcelino]

This pull request introduces TypeScript support.

[thatfiredev]

/gemini review

[gemini-code-assist]

Code Review

This pull request is a great step in modernizing the library by adding TypeScript support. The conversion has been done well, but it has also carried over some design issues from the original JavaScript code and introduced a few new areas for improvement. My main feedback is to refactor the library to be class-based to eliminate the use of a mutable global state for configuration, which is a critical issue for robustness and concurrent usage. I've also provided several other suggestions to improve type safety, error handling, and code consistency, such as using Error objects, avoiding any types, standardizing on camelCase naming, and cleaning up dependencies and configuration files. Addressing these points will result in a much more robust, maintainable, and professional library.

[gemini-code-assist]

The use of a mutable global variable mpesaConfig to store configuration is a significant design flaw in a library. It introduces global state, which is not safe for concurrent operations (e.g., in a web server handling multiple requests) and prevents using the library with multiple different configurations simultaneously. I strongly recommend refactoring to a class-based approach where configuration is managed per instance.

For example:

export class MpesaApi {
  private readonly config: MpesaConfig;

  constructor(config: MpesaConfig) {
    validateConfig(config);
    this.config = config;
  }

  public async initiateC2B(/*...*/) {
    // ... implementation using this.config
  }

  // ... other methods
}

[gemini-code-assist]

The code imports from the constants npm package, which is an old and unnecessary dependency. The required constants are available directly from Node.js's built-in crypto module. Using the built-in module is more secure and reduces the number of dependencies. You should remove this import and use crypto.constants.RSA_PKCS1_PADDING on line 23. After this change, you can remove the "constants": "0.0.2" dependency from package.json.

[gemini-code-assist]

This uses the constants object from the deprecated constants package. You should use the constants from the crypto module instead. This requires removing the import constants from 'constants'; on line 4.

Suggested change

	padding: constants.RSA_PKCS1_PADDING,
	padding: crypto.constants.RSA_PKCS1_PADDING,

[gemini-code-assist]

The use of optional chaining (?.) and nullish coalescing (|| '') on mpesaConfig suggests it might be undefined. If it were, this would lead to silent failures or obscure errors (e.g., sending an empty public key). The logic should instead guarantee that mpesaConfig is defined at this point, or fail loudly. A guard clause at the start of the function would be more robust.

// At the start of the function:
if (!mpesaConfig) {
    throw new Error("M-Pesa API not initialized. Call initializeApi() or set environment variables before making API calls.");
}

After adding the guard clause, you can safely remove the optional chaining operators.

[gemini-code-assist]

The function names like _getBearerToken, initialize_api_from_dotenv, required_config_arg, initiate_c2b, and initiate_b2c use snake_case. The common convention in the TypeScript/JavaScript community is camelCase (e.g., getBearerToken, initializeApiFromDotenv). For consistency within the project (initializeApi is already camelCase) and with the wider ecosystem, I recommend switching to camelCase.

Suggested change

	function _getBearerToken(mpesa_public_key: string, mpesa_api_key: string): string {
	function getBearerToken(mpesa_public_key: string, mpesa_api_key: string): string {

[gemini-code-assist]

The function validateConfig throws a string when validation fails. It's a best practice in JavaScript/TypeScript to throw Error objects, as they contain valuable information like stack traces and can be identified with instanceof Error.

Suggested change

	function validateConfig(configParams: MpesaConfig): void {
	if (!configParams.baseUrl) {
	throw required_config_arg("baseUrl");
	}
	if (!configParams.apiKey) {
	throw required_config_arg("apiKey");
	}
	if (!configParams.publicKey) {
	throw required_config_arg("publicKey");
	}
	if (!configParams.origin) {
	throw required_config_arg("origin");
	}
	if (!configParams.serviceProviderCode) {
	throw required_config_arg("serviceProviderCode");
	}
	}
	function validateConfig(configParams: MpesaConfig): void {
	if (!configParams.baseUrl) {
	throw new Error(required_config_arg("baseUrl"));
	}
	if (!configParams.apiKey) {
	throw new Error(required_config_arg("apiKey"));
	}
	if (!configParams.publicKey) {
	throw new Error(required_config_arg("publicKey"));
	}
	if (!configParams.origin) {
	throw new Error(required_config_arg("origin"));
	}
	if (!configParams.serviceProviderCode) {
	throw new Error(required_config_arg("serviceProviderCode"));
	}
	}

[gemini-code-assist]

The function returns a Promise<any>, which bypasses type checking for the response data. It's better to define an interface for the expected API response to provide type safety to consumers of your library. Also, consider renaming the function to initiateC2B to follow the camelCase convention.

Suggested change

	export const initiate_c2b = async (amount: number, msisdn: string, transaction_ref: string, thirdparty_ref: string): Promise<any> => {
	export const initiate_c2b = async (amount: number, msisdn: string, transaction_ref: string, thirdparty_ref: string): Promise<Record<string, any>> => {

[gemini-code-assist]

The catch block re-throws e.response.data, which is likely a plain object, not an Error instance. This breaks standard error handling conventions and loses the stack trace. It's better to wrap the response data in a new Error object.

    } catch (e: any) {
        if (axios.isAxiosError(e) && e.response?.data) {
            throw new Error(`M-Pesa API Error: ${JSON.stringify(e.response.data)}`);
        }
        throw e;
    }

[gemini-code-assist]

You have defined Mocha configuration both in this package.json file and in the new .mocharc.json file. This is redundant and can lead to confusion. It's best to keep the configuration in one place. I recommend keeping .mocharc.json as it's a dedicated file for this purpose, and removing this mocha block from package.json.

[gemini-code-assist]

The exclude array is missing the output directories (dist and types). While tsc usually doesn't re-process its own output, explicitly excluding these directories prevents potential issues, especially if build artifacts are not cleaned between runs.

  "exclude": [
    "node_modules",
    "dist",
    "types"
  ]