search

typed-typings / npm-ramda

TypeScript's type definitions for Ramda
MIT License
384 stars 64 forks source link

Use prepack instead of prepublish #389

Closed knpwrs closed 6 years ago

knpwrs commented 6 years ago

prepublish is no longer the correct way to build before publish. The correct script to use is prepack. The reason for this is that prepublish runs after pack and before publish, and pack is the script which defines what is published during publish, i.e., the package being published is already created by the time prepublish is called. By building in prepack the package will be created properly during pack and then published during publish. Using prepublish relies on having an already-done, possibly stale build.

KiaraGrouwstra commented 6 years ago

sounds good, thanks for the PR! not sure what CI is yapping about. @ikatyang?

ikatyang commented 6 years ago

We use prepublish to build types after npm install, you probably have to add yarn run build to .travis.yml as well.

knpwrs commented 6 years ago

Yep, that appears to have fixed it!

ikatyang commented 6 years ago

Thanks!

Draeken commented 6 years ago

So, yarn is mandatory if I want to use @types/ramda ?

ikatyang commented 6 years ago

No, see Usage.

Draeken commented 6 years ago

Yes, but when performing npm install --save-dev types/npm-ramda#dist I get:

> @types/ramda@0.25.0 prepack /Users/***/.npm/_cacache/tmp/git-clone-73a2dbb0`
> yarn run build

sh: yarn: command not found
ikatyang commented 6 years ago

Ah, sorry. I didn't notice that'll trigger the prepack script after installing from git. Reverted in bf2c978.

prepack: run BEFORE a tarball is packed (on npm pack, npm publish, and when installing git dependencies)

knpwrs commented 6 years ago

@ikatyang would it possibly be best to change the package.json script to use npm instead of yarn? As it stood previously, prepublish would require yarn. Not particularly a big deal if all the maintainers use yarn. Ultimately the change to use the prepack script is meant to reduce the chance of a possible error getting published.

ikatyang commented 6 years ago

Actually, the problem is that we don't need to build from source, the files from dist branch are already built. And currently, we do not publish anything to npm since @types namespace are handled by DefinitelyTyped.

KiaraGrouwstra commented 6 years ago

I don't know all the considerations for yarn, but I think recent npm releases did fix much of their original issues.