Skip to content

Commit

Permalink
refactor: bring back json imports (#1176)
Browse files Browse the repository at this point in the history
## 🧰 Changes

[node v20.18.3](https://nodejs.org/en/blog/release/v20.18.3) officially
marked import attributes [as
stable](https://nodejs.org/docs/latest-v20.x/api/esm.html#import-attributes).
we have a lot of weird workarounds with our `oclif` setup in this repo
in an effort to avoid import attributes and the ugly
`ExperimentalWarning` outputs we see, but i don't think those should be
a concern anymore, for a few reasons:

- our minimum required node.js version is node 20, so it should be a
non-disruptive change for users to upgrade to the latest node 20 channel
- even if folks are on <20.18.2, the `ExperimentalWarning` outputs log
to `stderr` so it shouldn't affect any command output-based workflows,
which almost always are relying on `stdout`. plus, we discourage this
kind of scripting and recommend folks use exit codes instead

given the above, this PR brings back JSON imports of `package.json` and
vastly simplifies a lot of weirdness in this codebase.

## 🧬 QA & Testing

we've added a lot of good test coverage now and i've been playing around
with the CLI and github actions build locally and everything seems to
work as expected.

very unlikely we'll run into the fiasco in
#1117, especially now with the
cleanups in this PR and #1172.
  • Loading branch information
kanadgupta authored Feb 28, 2025
1 parent cd73667 commit bd47b89
Show file tree
Hide file tree
Showing 7 changed files with 6 additions and 24 deletions.
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -6,4 +6,3 @@ node_modules/
# required for building with TS + oclif + GitHub Actions
dist-gha/package.json
oclif.manifest.json
src/package.json
6 changes: 0 additions & 6 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,6 @@

## Running Shell Commands Locally 🐚

To get started, run the `build` script to create a symlink with `package.json` (required for our `oclif` setup to read our commands properly). You only need to do this the first time you clone the repository.

```sh
npm run build
```

To run test commands, swap out `rdme` for `bin/dev.js`. For example:

```sh
Expand Down
2 changes: 1 addition & 1 deletion bin/dev.cmd
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
@echo off

node --no-warnings=ExperimentalWarning "%~dp0\dev" %*
node "%~dp0\dev" %*
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -109,12 +109,12 @@
"vitest": "^3.0.0"
},
"scripts": {
"build": "tsc && cp package.json dist/package.json",
"build": "tsc",
"build:docs": "oclif readme --multi --output-dir documentation/commands --no-aliases",
"build:gha": "npm run build && rollup --config",
"lint": "alex . && knip && npm run lint:ts && npm run prettier && npm run schemas:check",
"lint:ts": "eslint . --ext .js,.ts",
"prebuild": "rm -rf dist/ && rm -f src/package.json && ln package.json src/package.json",
"prebuild": "rm -rf dist/",
"prepack": "npm run build",
"prepare": "husky",
"pretest": "npm run 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 './getPkg.js';
import pkg from '../../package.json' with { type: 'json' };

const configstore = new Configstore(
/**
Expand Down
13 changes: 2 additions & 11 deletions src/lib/getPkg.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import type { Hook } from '@oclif/core';

import { readFileSync } from 'node:fs';

import semver from 'semver';

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

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

const registryUrl = 'https://registry.npmjs.com/rdme';
Expand All @@ -13,15 +13,6 @@ 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 Down
2 changes: 0 additions & 2 deletions vitest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@ export default defineConfig({
* tool in a testing environment.
*/
NODE_ENV: 'rdme-test',
// Node emits ExperimentalWarnings because we import JSON modules, so this hides that output.
NODE_OPTIONS: '--disable-warning=ExperimentalWarning',
},
exclude: [
'**/__fixtures__/**',
Expand Down

0 comments on commit bd47b89

Please sign in to comment.