yeoman / update-notifier

Update notifications for your CLI app
BSD 2-Clause "Simplified" License
1.76k stars 132 forks source link

Default `pkg` to the current package #172

Open ehmicky opened 4 years ago

ehmicky commented 4 years ago

The pkg option must be set with the package's package.json.

However this does not work well with ES imports. At the moment, importing JSON files with ES imports is still experimental in Node.js.

Also, hardcoding the path to package.json can be tricky when transpiling is involved. For example, my binary file is at src/bin.js, but it is transpiled to build/src/bin.js. If I want to run both files, I need to dynamically find the path to package.json with a library like read-pkg-up.

Adding an option to automatically find the package (with read-pkg-up for example) would solve this problem. This option would probably need user to pass __dirname to be able to find the right package.json.

What do you think?

I can submit a PR if needed.

sindresorhus commented 4 years ago

Sure. To be ESM compatible, it should accept a URL: https://nodejs.org/api/esm.html#esm_import_meta __dirname doesn't exist in ESM modules.

ehmicky commented 4 years ago

Trying to figure out the shape of this. What about: allowing the pkg option to be either a plain object (current behavior) or a URL? Also I think both a URL and a file path string should be allowed to be both ESM and CommonJS compatible.

sindresorhus commented 4 years ago

What about: allowing the pkg option to be either a plain object (current behavior) or a URL?

👍

Also I think both a URL and a file path string should be allowed to be both ESM and CommonJS compatible.

I disagree. CommonJS users can already just use require('./package.json'). I would prefer to limit the types to only what's necessary.

astorije commented 4 years ago

I found this after experiencing the same issue and coming to the same conclusion(s)! 😅 For additional context, transpiling copies package.json to dist/package.json, but our CI tooling then publishes packages with (yarn|npm) (version|publish) which updates version only in the top-level package.json, catch-22!

I would love if updateNotifier() automatically loaded the right package.json (e.g. with read-pkg-up), but in the meantime, these are my workarounds, summed up in a heavily simplified diff, in case it is useful to anyone:

diff --git a/index.ts b/index.ts
 #!/usr/bin/env node

+// eslint-disable-next-line import/default
+import readPkgUp from 'read-pkg-up';
 import updateNotifier from 'update-notifier';

-import packageJson from './package.json';

+const { packageJson } = readPkgUp.sync() ?? {};
+
+if (packageJson) {
   updateNotifier({ pkg: packageJson }).notify();
+}

diff --git a/package.json b/package.json
+    "read-pkg-up": "7.0.1",

diff --git a/tsconfig.json b/tsconfig.json
-    "resolveJsonModule": true, // Include modules imported with `.json` extension.

Workarounds, plural, because I'm debating with a much simpler alternative at the moment... It feels a little more hacky and more ad-hoc to my setup, but seems simpler:

diff --git a/package.json b/package.json
   "scripts": {
+    "build": "tsc --build && ln -sf ../package.json dist/package.json",

Not sure which route to go for. Future-and-less-sleep-deprived-me will make the right decision.

equiman commented 2 years ago

I'm reading the package information on my own

import { readFile } from 'fs/promises'

export const getPackageInfo = async () => {
  const pkg = await readFile(new URL('../../package.json', import.meta.url))
  return JSON.parse(pkg)
}

And using to send this information as pkg parameter

import updateNotifier from 'update-notifier'
import { getPackageInfo } from './info.js'

export const autoUpdate = async () => {
  const { name, version } = await getPackageInfo()

  const option = {
    pkg: {
      name,
      version
    },
    shouldNotifyInNpmScript: true,
    updateCheckInterval: 0
  }
  const notifier = updateNotifier(option)
  await notifier.notify()
}

And call it from index.js

import { autoUpdate } from './helpers/autoUpdate.js'
await autoUpdate()

Sadly is using an await but is the only way that I found works.