Skip to content

Commit

Permalink
fix: remove import attributes (#1117)
Browse files Browse the repository at this point in the history
## 🧰 Changes

The whole reasoning behind #1115 is because we pass flags in order to
avoid emitting this `ExperimentalWarning` for Node v20 users:

![CleanShot 2024-12-10 at 12 12
00@2x](https://github.com/user-attachments/assets/de75f8ff-06d5-43c2-be44-ddedba60187e)

Per [this
comment](#1115 (comment)),
adding `-S` might be problematic for certain container setups. As a
result, I just overhauled how we import JSON in our production code so
we don't need that Node CLI flag at all.

At some point I'd love if we got to a point where we can fully rely on
the oclif config for `package.json` attributes but today is not that
day!

## 🧬 QA & Testing

When you check out this branch and run the following commands...

- Do they run properly without hanging? (hopefully yes)
- Do you see Node `ExperimentalWarning` outputs (like the above
screenshot) at any point? (hopefully no)


```sh
npm ci && npm run build
bin/run.js
```
  • Loading branch information
kanadgupta authored Dec 10, 2024
1 parent 0fb7880 commit e662654
Show file tree
Hide file tree
Showing 14 changed files with 61 additions and 26 deletions.
17 changes: 17 additions & 0 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -48,3 +48,20 @@ jobs:
with:
github_token: ${{ secrets.RELEASE_GH_TOKEN }}
branch: next

# quick assertion to validate that rdme CLI can be installed and run on ubuntu
postrelease:
name: Post-release checks
needs: release
runs-on: ubuntu-latest
if: ${{ github.ref }} == 'refs/heads/next'
steps:
- name: Set up Node.js
uses: actions/setup-node@v4
with:
node-version: lts/*
- name: Install `rdme` from npm
run: npm install -g rdme@next
- name: Print rdme CLI version
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 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 e662654

Please sign in to comment.