Skip to content

Commit

Permalink
fix: bring back #1117 without breaking everything (#1120)
Browse files Browse the repository at this point in the history
## 🧰 Changes

we hit a bit of a wild edge case where the github action was only
breaking in production builds due to how oclif loads the `package.json`
and the only way we caught it was with [this
failure](https://github.com/readmeio/rdme/actions/runs/12267183540/job/34226921877).

this PR reverts #1119 (which in
turn brings back #1117) with a
slight tweak in
f9461b4
to do the following:
- make our import paths friendlier to github actions
- manually copy over the `package.json` to our `dist/` directory
whenever we run `npm run build`. TS was automatically handling this when
we were using JSON imports before but now it's not able to pick up on
the import so we have to copy it over ourselves.

additionally in
669cb4f,
i added a little check so we can catch these sorts of things better
going forward.

## 🧬 QA & Testing

Provide as much information as you can on how to test what you've done.
  • Loading branch information
kanadgupta authored Dec 11, 2024
1 parent 73dbc17 commit d5d74c5
Show file tree
Hide file tree
Showing 16 changed files with 62 additions and 27 deletions.
8 changes: 8 additions & 0 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -113,3 +113,11 @@ jobs:
uses: ./rdme-repo/
with:
rdme: openapi "oas-examples-repo/3.1/json/petstore.json" --key "${{ secrets.RDME_TEST_PROJECT_API_KEY }}" --id=${{ secrets.RDME_TEST_PROJECT_API_SETTING }}

# this is a test to ensure that the rdme github action can run properly
# the way that our users invoke it
- name: E2E run of `openapi validate` on `next` branch
uses: readmeio/rdme@next
if: ${{ github.ref }} == 'refs/heads/next'
with:
rdme: openapi validate oas-examples-repo/3.1/json/petstore.json
9 changes: 9 additions & 0 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,12 @@ jobs:
with:
github_token: ${{ secrets.RELEASE_GH_TOKEN }}
branch: next

# quick assertion to validate that rdme CLI can be installed and run on ubuntu
- name: Install `rdme` from npm
if: ${{ github.ref }} == 'refs/heads/next'
run: npm install -g rdme@next
- name: Print rdme CLI version
if: ${{ github.ref }} == 'refs/heads/next'
run: rdme --version
timeout-minutes: 1
2 changes: 1 addition & 1 deletion __tests__/commands/logout.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { describe, afterEach, beforeAll, it, expect } from 'vitest';

import pkg from '../../package.json';
import pkg from '../../package.json' with { type: 'json' };
import Command from '../../src/commands/logout.js';
import configStore from '../../src/lib/configstore.js';
import { runCommandAndReturnResult } from '../helpers/oclif.js';
Expand Down
2 changes: 1 addition & 1 deletion __tests__/commands/open.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import type { Version } from '../../src/commands/versions/index.js';
import chalk from 'chalk';
import { describe, afterEach, beforeAll, it, expect } from 'vitest';

import pkg from '../../package.json';
import pkg from '../../package.json' with { type: 'json' };
import Command from '../../src/commands/open.js';
import configStore from '../../src/lib/configstore.js';
import { getAPIv1Mock } from '../helpers/get-api-mock.js';
Expand Down
2 changes: 1 addition & 1 deletion __tests__/commands/whoami.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { describe, afterEach, it, expect, beforeAll } from 'vitest';

import pkg from '../../package.json';
import pkg from '../../package.json' with { type: 'json' };
import Command from '../../src/commands/whoami.js';
import configStore from '../../src/lib/configstore.js';
import { runCommandAndReturnResult } from '../helpers/oclif.js';
Expand Down
2 changes: 1 addition & 1 deletion __tests__/helpers/get-gha-setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { vi } from 'vitest';

import configstore from '../../src/lib/configstore.js';
import { git } from '../../src/lib/createGHA/index.js';
import * as getPkgVersion from '../../src/lib/getPkgVersion.js';
import * as getPkgVersion from '../../src/lib/getPkg.js';

import getGitRemoteMock from './get-git-mock.js';

Expand Down
2 changes: 1 addition & 1 deletion __tests__/lib/createGHA.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { describe, beforeEach, afterEach, it, expect, vi, type MockInstance, bef

import configstore from '../../src/lib/configstore.js';
import { getConfigStoreKey, getGHAFileName, git } from '../../src/lib/createGHA/index.js';
import { getMajorPkgVersion } from '../../src/lib/getPkgVersion.js';
import { getMajorPkgVersion } from '../../src/lib/getPkg.js';
import { after, before } from '../helpers/get-gha-setup.js';
import getGitRemoteMock from '../helpers/get-git-mock.js';
import ghaWorkflowSchema from '../helpers/github-workflow-schema.json' with { type: 'json' };
Expand Down
8 changes: 4 additions & 4 deletions __tests__/lib/getPkgVersion.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import semver from 'semver';
import { describe, beforeEach, afterEach, it, expect, vi, type MockInstance } from 'vitest';

import pkg from '../../package.json' with { type: 'json' };
import { getNodeVersion, getPkgVersion } from '../../src/lib/getPkgVersion.js';
import { getNodeVersion, getPkgVersion, getPkgVersionFromNPM } from '../../src/lib/getPkg.js';

describe('#getNodeVersion()', () => {
it('should extract version that matches range in package.json', () => {
Expand All @@ -27,23 +27,23 @@ describe('#getPkgVersion()', () => {
});

it('should grab version from package.json by default', () => {
return expect(getPkgVersion()).resolves.toBe(pkg.version);
return expect(getPkgVersion()).toBe(pkg.version);
});

it('should fetch version from npm registry', async () => {
const mock = nock('https://registry.npmjs.com', { encodedQueryParams: true })
.get('/rdme')
.reply(200, { 'dist-tags': { latest: '1.0' } });

await expect(getPkgVersion('latest')).resolves.toBe('1.0');
await expect(getPkgVersionFromNPM('latest')).resolves.toBe('1.0');

mock.done();
});

it('should fallback if npm registry fails', async () => {
const mock = nock('https://registry.npmjs.com', { encodedQueryParams: true }).get('/rdme').reply(500);

await expect(getPkgVersion('latest')).resolves.toBe(pkg.version);
await expect(getPkgVersionFromNPM('latest')).resolves.toBe(pkg.version);

mock.done();
});
Expand Down
2 changes: 1 addition & 1 deletion bin/dev.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#!/usr/bin/env -S npx tsx
#!/usr/bin/env npx tsx

async function main() {
const { execute } = await import('@oclif/core');
Expand Down
4 changes: 1 addition & 3 deletions bin/run.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
#!/usr/bin/env -S node --no-warnings=ExperimentalWarning
// ^ we need this env variable above to hide the ExperimentalWarnings
// source: https://github.com/nodejs/node/issues/10802#issuecomment-573376999
#!/usr/bin/env node

import stringArgv from 'string-argv';

Expand Down
2 changes: 1 addition & 1 deletion bin/set-version-output.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import * as core from '@actions/core';

// eslint-disable-next-line import/extensions
import { getNodeVersion, getMajorPkgVersion } from '../dist/lib/getPkgVersion.js';
import { getNodeVersion, getMajorPkgVersion } from '../dist/lib/getPkg.js';

/**
* Sets output parameters for GitHub Actions workflow so we can do
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@
"vitest": "^2.0.5"
},
"scripts": {
"build": "tsc",
"build": "tsc && ln package.json dist/package.json",
"build:docs": "oclif readme --multi --output-dir documentation/commands --no-aliases",
"build:gha": "npm run build && rollup --config",
"build:gha:prod": "npm run build:gha -- --environment PRODUCTION_BUILD",
Expand Down
2 changes: 1 addition & 1 deletion src/lib/configstore.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import Configstore from 'configstore';

import pkg from '../package.json' with { type: 'json' };
import { pkg } from './getPkg.js';

const configstore = new Configstore(
/**
Expand Down
2 changes: 1 addition & 1 deletion src/lib/createGHA/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import prompts from 'prompts';
import { simpleGit } from 'simple-git';

import configstore from '../configstore.js';
import { getMajorPkgVersion } from '../getPkgVersion.js';
import { getMajorPkgVersion } from '../getPkg.js';
import isCI, { isNpmScript, isTest } from '../isCI.js';
import { info } from '../logger.js';
import promptTerminal from '../promptWrapper.js';
Expand Down
35 changes: 28 additions & 7 deletions src/lib/getPkgVersion.ts → src/lib/getPkg.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import type { Hook } from '@oclif/core';

import semver from 'semver';
import { readFileSync } from 'node:fs';

import pkg from '../package.json' with { type: 'json' };
import semver from 'semver';

import { error } from './logger.js';

Expand All @@ -13,6 +13,15 @@ const registryUrl = 'https://registry.npmjs.com/rdme';
*/
type npmDistTag = 'latest';

/**
* A synchronous function that reads the `package.json` file for use elsewhere.
* Until we drop support Node.js 20, we need to import this way to avoid ExperimentalWarning outputs.
*
* @see {@link https://nodejs.org/docs/latest-v20.x/api/esm.html#import-attributes}
* @see {@link https://www.stefanjudis.com/snippets/how-to-import-json-files-in-es-modules-node-js/}
*/
export const pkg = JSON.parse(readFileSync(new URL('../package.json', import.meta.url), { encoding: 'utf-8' }));

/**
* Return the major Node.js version specified in our `package.json` config.
*
Expand All @@ -27,27 +36,39 @@ export function getNodeVersion(): string {
return parsedVersion.version;
}

/**
* The current `rdme` version, as specified in the `package.json`
* or in the oclif hook context.
*
* @example "8.0.0"
* @note we mock this function in our snapshots
* @see {@link https://stackoverflow.com/a/54245672}
*/
export function getPkgVersion(this: Hook.Context | void): string {
return this?.config?.version || pkg.version;
}

/**
* The current `rdme` version
*
* @param npmDistTag the `npm` dist tag to retrieve. If this value is omitted,
* the version from the `package.json` is returned.
* @example "8.0.0"
* @see {@link https://docs.npmjs.com/adding-dist-tags-to-packages}
* @note we mock this function in our snapshots, hence it's not the default
* @note we mock this function in our snapshots
* @see {@link https://stackoverflow.com/a/54245672}
*/
export async function getPkgVersion(this: Hook.Context | void, npmDistTag?: npmDistTag): Promise<string> {
export async function getPkgVersionFromNPM(this: Hook.Context | void, npmDistTag?: npmDistTag): Promise<string> {
if (npmDistTag) {
return fetch(registryUrl)
.then(res => res.json() as Promise<{ 'dist-tags': Record<string, string> }>)
.then(body => body['dist-tags'][npmDistTag])
.catch(err => {
error(`error fetching version from npm registry: ${err.message}`);
return pkg.version;
return getPkgVersion.call(this);
});
}
return this?.config?.version || pkg.version;
return getPkgVersion.call(this);
}

/**
Expand All @@ -56,5 +77,5 @@ export async function getPkgVersion(this: Hook.Context | void, npmDistTag?: npmD
* @example 8
*/
export async function getMajorPkgVersion(this: Hook.Context | void, npmDistTag?: npmDistTag): Promise<number> {
return semver.major(await getPkgVersion.call(this, npmDistTag));
return semver.major(await getPkgVersionFromNPM.call(this, npmDistTag));
}
5 changes: 2 additions & 3 deletions src/lib/readmeAPIFetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,10 @@ import path from 'node:path';
import mime from 'mime-types';
import { ProxyAgent } from 'undici';

import pkg from '../package.json' with { type: 'json' };

import { APIv1Error } from './apiError.js';
import config from './config.js';
import { git } from './createGHA/index.js';
import { getPkgVersion } from './getPkg.js';
import isCI, { ciName, isGHA } from './isCI.js';
import { debug, warn } from './logger.js';

Expand Down Expand Up @@ -90,7 +89,7 @@ function parseWarningHeader(header: string): WarningHeader[] {
*/
export function getUserAgent() {
const gh = isGHA() ? '-github' : '';
return `rdme${gh}/${pkg.version}`;
return `rdme${gh}/${getPkgVersion()}`;
}

/**
Expand Down

0 comments on commit d5d74c5

Please sign in to comment.