search

nodejs / repl

REPL rewrite for Node.js ✨🐢🚀✨
MIT License
177 stars 25 forks source link

Simplify installation #28

Closed geppy closed 5 years ago

geppy commented 5 years ago

Add prepare npm script that's run when npm install is run locally with no commands (it'll also run before the package is packed and published).

I've also updated the installation instructions, since @nodejs/repl hasn't been published to the public npm registry yet.

devsnek commented 5 years ago

the annotations are already generated. also, you refer to it as @nodejs/repl when it should be nodejs/repl.

geppy commented 5 years ago

the annotations are already generated.

The annotations in the repository didn't work on my machine, since some platform-specific APIs were missing (and undefined isn't a valid WeakMap key).

Would you consider a separate PR that filters out missing APIs at runtime? For example, fs.lchmod and fs.lchmodSync are “only available on macOS”.

also, you refer to it as @nodejs/repl when it should be nodejs/repl.

I was using the npm package name (@nodejs/repl, set in package.json), since the existing instructions say to install this from the npm registry (via npm install nodejs/repl). @nodejs/repl is a valid package name (using the package scope @nodejs, but nodejs/repl is not (a package name “must be lowercase and one word, and may contain hyphens and underscores”).

If this won't be published to the registry, nodejs/repl makes sense. Do you want this changed to nodejs/repl?

devsnek commented 5 years ago

@geppy

The annotations in the repository didn't work on my machine, since some platform-specific APIs were missing (and undefined isn't a valid WeakMap key).

I see, i wasn't aware of this. Then yes i would agree a change needs to be made.

the existing instructions say to install this from the npm registry (via npm install nodejs/repl)

Using npm i -g nodejs/repl (without the @) will install directly from github, which is my intention here. Using @nodejs/repl is set because its just the npm scope we use in node, so i don't think it needs to be changed.

I don't want to force additional dependencies (typescript is rather big, etc) so i think adding some sort of filter to the generated output would make sense.

geppy commented 5 years ago

Using npm i -g nodejs/repl (without the @) will install directly from github

Oh. Thanks! I didn't know that.

Sorry for the noise.