Skip to content

Docs(ReadMe): Adding note for installing CLI as root - #1343

Closed
rschristian wants to merge 1 commit into
preactjs:masterfrom
rschristian:master
Closed

Docs(ReadMe): Adding note for installing CLI as root #1343
rschristian wants to merge 1 commit into
preactjs:masterfrom
rschristian:master

Conversation

@rschristian

Copy link
Copy Markdown
Member

What kind of change does this PR introduce?

Adding documentation to cover (what might become) a common issue.

Did you add tests for your changes?

N/A

Summary

Now that the default install for the CLI is v3.0.0, more people will probably run into the same problem that is presented in #802 and #1342. While running NPM as root, the unsafe-perm flag will be set to false by default which causes an issue with (at least) nodent-runtime. NPM docs, nodent-runtime. The simple fix is to add the unsafe-perm flag.

As running global installations as root is a pretty common thing to do, having documentation to catch the people who run into this issue is likely worthwhile.

Does this PR introduce a breaking change?

No breaking changes

@developit

developit commented Jul 24, 2020

Copy link
Copy Markdown
Member

I'm (very) hesitant to include anything about running as root in the readme. 99.99% of the time it's danger without justification.

Perhaps we can drop nodent-runtime. I'm actually not even sure it's being used? We switched from fast-async to transform-async-to-promises ages ago. Edit: we did not switch. We should:

[require.resolve('fast-async'), { spec: true }],

Now that the CLI does modern output via module/nomodule by default, there's no reason to use fast-async and we can switch to Babel's default transform-async-to-gen + regenerator, with regenerator being loaded in the <script nomodule src="polyfills.js"> chunk.

@rschristian

Copy link
Copy Markdown
Member Author

That's certainly reasonable.

I only mention it as it is the default on non-Windows systems (assuming the dev does a simple Node/NPM install without a Node version manager) to need root for global installs. It's entirely anecdotal and without any numbers backing it, but I feel like I've seen rather large amount of libs/tools telling users to just use sudo. That sort of advice in addition to it "working" (*with potential security risks) has led me to believe the majority probably do just use sudo rather than a version manager or moving NPM's default directory.

Could totally be wrong. Also might not be worth showing the easy way around EACCES issues and instead having them find the NPM documentation showing the proper way around instead.

@teodragovic

teodragovic commented Jul 25, 2020

Copy link
Copy Markdown
Contributor

How about updating docs to use npx preact create instead?

@teodragovic teodragovic mentioned this pull request Jul 25, 2020
@rschristian

Copy link
Copy Markdown
Member Author

Certainly could do.

CRA has been telling users to use npx over global installs for a while now, to ensure they're always up to date. CRA as a global dependency fills the same area that the Preact-CLI does, so maybe it's worth doing the same here?

Npx certainly is slower but that might be a better suggestion going forward. A bit slower of initializing a new project every now and then probably won't be noticeable for most users.

@ForsakenHarmony

Copy link
Copy Markdown
Member

the majority probably do just use sudo rather than a version manager or moving NPM's default directory.

Honestly that's on npm, and they should really change the default directory

@rschristian

Copy link
Copy Markdown
Member Author

Honestly that's on npm, and they should really change the default directory

Oh definitely. However, until they do, people will probably keep using sudo for their global installs, which comes with some issues for build scripts that the Preact CLI employs (at least for now).

This PR is just here in case you do want to catch the few people who will run into issues, as undoubtedly some percentage will. I could amend this to discourage sudo use, or perhaps this should just be closed without any additional documentation?

Not my repository by any means, so let me know if you'd like for me to change something or if this isn't what the team wants to encourage.

@rschristian

Copy link
Copy Markdown
Member Author

Closing as better alternatives have been pointed out, so I doubt this is going anywhere or worth keeping.

teodragovic added a commit to teodragovic/preact-cli that referenced this pull request Jul 30, 2020
This change recommends using npx to avoid global installs of `preact-cli`

Related discussions: preactjs#1343 and preactjs#1342
prateekbh pushed a commit that referenced this pull request Aug 4, 2020
* Update install instructions to use npx

This change recommends using npx to avoid global installs of `preact-cli`

Related discussions: #1343 and #1342

* Update README.md

Co-authored-by: Ryan Christian <33403762+RyanChristian4427@users.noreply.github.com>

* Fix package name

Co-authored-by: Ryan Christian <33403762+RyanChristian4427@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants