Include Typescript as a dependency or peerDependency

jeffsee55 commented 4 years ago

When using yarn (v2) pnp and building types with the --dts command we get the following error

error: rollup-plugin-dts tried to access typescript (a peer dependency) but it isn't provided by its ancestors; this makes the require call ambiguous and unsound.

I can see why typescript isn't a dependency since it's only used when generating types - perhaps it's best to just list it in peerDependencies - though I imagine it'd be best to permit ^3.9.7 as well as anything above ^4.0.0 if that's the case. Happy to submit a PR for this.

egoist commented 3 years ago

:ok: sounds good to me

privatenumber commented 3 years ago

Seems like it's already implicitly a peer dependency:

➜  mdu git:(master) ✗ npm run build

> mdu@0.0.0-semantic-release build /Users/pvtnbr/Documents/public-github/privatenumber/mdu
> tsup src/mdu.ts --dts --format esm

 CLI  Building entry: src/mdu.ts
 CLI  Target: es2018
 ESM  Build start
 ESM  Build success in 59ms

events.js:292
      throw er; // Unhandled 'error' event
      ^
Error: Cannot find module 'typescript'
Require stack:
- /Users/pvtnbr/Documents/public-github/privatenumber/mdu/node_modules/.pnpm/rollup-plugin-dts@2.0.1_rollup@2.40.0/node_modules/rollup-plugin-dts/dist/rollup-plugin-dts.js
- /Users/pvtnbr/Documents/public-github/privatenumber/mdu/node_modules/.pnpm/tsup@4.6.0/node_modules/tsup/dist/rollup.js
    at Function.Module._resolveFilename (internal/modules/cjs/loader.js:880:15)
    at Function.Module._load (internal/modules/cjs/loader.js:725:27)
    at Module.require (internal/modules/cjs/loader.js:952:19)
    at require (internal/modules/cjs/helpers.js:88:18)
    at Object.<anonymous> (/Users/pvtnbr/Documents/public-github/privatenumber/mdu/node_modules/.pnpm/rollup-plugin-dts@2.0.1_rollup@2.40.0/node_modules/rollup-plugin-dts/dist/rollup-plugin-dts.js:6:10)
    at Module._compile (internal/modules/cjs/loader.js:1063:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1092:10)
    at Module.load (internal/modules/cjs/loader.js:928:32)
    at Function.Module._load (internal/modules/cjs/loader.js:769:14)
    at Module.require (internal/modules/cjs/loader.js:952:19)
Emitted 'error' event on process instance at:
    at emitUnhandledRejectionOrErr (internal/event_target.js:545:11)
    at MessagePort.[nodejs.internal.kHybridDispatch] (internal/event_target.js:358:9)
    at MessagePort.exports.emitMessage (internal/per_context/messageport.js:18:26) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/Users/pvtnbr/Documents/public-github/privatenumber/mdu/node_modules/.pnpm/rollup-plugin-dts@2.0.1_rollup@2.40.0/node_modules/rollup-plugin-dts/dist/rollup-plugin-dts.js',
    '/Users/pvtnbr/Documents/public-github/privatenumber/mdu/node_modules/.pnpm/tsup@4.6.0/node_modules/tsup/dist/rollup.js'
  ]
}
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! mdu@0.0.0-semantic-release build: `tsup src/mdu.ts --dts --format esm`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the mdu@0.0.0-semantic-release build script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/pvtnbr/.npm/_logs/2021-03-03T05_12_23_109Z-debug.log

egoist commented 3 years ago

Added as an optional peerDependency in 4.6.1, it will throw an error if typescript is not installed when using the --dts flag:

egoist / tsup

Include Typescript as a dependency or peerDependency #160