graphile / starter

Opinionated SaaS quick-start with pre-built user account and organization system for full-stack application development in React, Node.js, GraphQL and PostgreSQL. Powered by PostGraphile, TypeScript, Apollo Client, Graphile Worker, Graphile Migrate, GraphQL Code Generator, Ant Design and Next.js
https://graphile-starter.herokuapp.com
Other
1.74k stars 219 forks source link

fix: remove npx -n usage in favor of cross-env / NODE_OPTIONS #230

Closed DylanVann closed 3 years ago

DylanVann commented 3 years ago

Description

On cloning this repo and following instructions I encountered errors saying -n is no longer supported in npx.

image

It seems that my version of npx is too old to work correctly. I normally use fnm to manage Node versions and don't use npm or npx too often so I hadn't noticed this.

More detail: I've installed Node 15.5.1 using fnm, this installs npm@7.3.0 and npx@7.3.0, I searched online but I don't see any matrix mapping npm version -> npx version anywhere, maybe fnm is doing something incorrect by using the same version as npm.

This could be mitigated for other users by using NODE_OPTIONS instead of npx.

Performance impact

Possibly slightly better perf 🤷‍♂️ , most of these commands were already using cross-env.

Security impact

None.

Checklist

benjie commented 3 years ago

Do you know if this still works on Windows? That’s why we had to do the npx stuff iirc.

DylanVann commented 3 years ago

I believe it should because it's making use of cross-env. NODE_OPTIONS should work on Windows.

After trying to get a Windows dev environment working in VirtualBox for over 2 hours I'm calling it quits, maybe someone else can test.

Not sure what it does exactly, but the fact that "Windows Node CI" is passing seems to be a good sign.

Edit: Looking at what "Windows Node CI" is doing, I'm 99% sure it would not be passing if NODE_OPTIONS was not working correctly.

benjie commented 3 years ago

Awesome; thanks for checking @DylanVann. We seem to be having some CI issue but they look transient, so I'll trigger a rebuild and see if we can get them to pass.

DylanVann commented 3 years ago

That did the trick, thanks!

benjie commented 3 years ago

Awesome; thanks! :raised_hands: