apollographql / graphql-subscriptions

:newspaper: A small module that implements GraphQL subscriptions for Node.js
MIT License
1.58k stars 133 forks source link

fix: installing via git not creating dist #243

Closed OmgImAlexis closed 3 years ago

OmgImAlexis commented 3 years ago

When installing via GitHub e.g. npm i github:apollographql/graphql-subscriptions#master the dist directory isn't created as the prepare script is missing. This add it.

OmgImAlexis commented 3 years ago

This is currently blocking me from fully testing https://github.com/apollographql/graphql-subscriptions/pull/209 as I can't install the fork as it doesn't run compile on install.

OmgImAlexis commented 3 years ago

@glasser this should be good to review.

glasser commented 3 years ago

Thanks, see my comment there re my schedule.

glasser commented 3 years ago

Would it make more sense to rename the current prepublishOnly script to prepare? Otherwise I think it will compile twice on publish, right?

OmgImAlexis commented 3 years ago

I believe it should stay this way.

As of npm@5, prepublish scripts are deprecated. Use prepare for build steps and prepublishOnly for upload-only. See the deprecation note in npm help scripts for more information.

glasser commented 3 years ago

I'm not proposing adding the deprecated publish though, just the prepare which I believe is accurately used for both.

I'll give my version a shot and if it doesn't work out then we can try yours later!

glasser commented 3 years ago

I merged my version. Let me know if it doesn't solve your problem.