billiegoose / modserv

A next generation registry (WIP)
Apache License 2.0
9 stars 1 forks source link

Support for subdomains and routing configurability in general #2

Closed billiegoose closed 7 years ago

billiegoose commented 7 years ago

This discussion originally took place on https://github.com/nolanlawson/local-npm/pull/127 and is duplicated here since it had some important notes about the state of the code when I was last working on it

(This isn't ready to be merged. I'm just opening it now for discussion.)

The Express server has some hard-coded mounting points, such as /_browse, /_skimdb, and /_utils. I wanted to mount the UI at browse.node-modules.io, the registry at registry.node-modules.io, etc, so I'm adding that ability here. Also, I wanted to be able to specify all these paths and ports in a config file rather than as command line flags, so I'm adding that.

It appears to work as I open this pull request, but I'm not satisfied with it. I think I can make it a lot simpler, possibly even splitting the UI, registry, and tarball services in such a way that they could be individually enabled/disabled so you could (for example) run the UI on one node, have the tarballs routed through a CDN, and run the registry on a separate machine and a different port. Something like:

@node1:~$ local-npm --browse=http://browse.node-modules.io --tarballs=http://cdn.node-modules.io --port=8080
@node2:~$ local-npm --registry=https://registry.node-modules.io:5858 --port=5858

@zeke Ultimately, splitting the code cleanly into modules by separating concerns will make it easier to do crazy things, like support alternative backends (IPFS?).

billiegoose commented 7 years ago

Then @zeke said:

I don't know much about this codebase so I won't comment on the implementation, but I like this idea:

local-npm --browse=http://browse.node-modules.io --tarballs=http://cdn.node-modules.io --port=8080

Then @nolanlawson said:

Hey, thanks for the PR, but this repo is actually unmaintained, and this seems like kind of an ambitious change in behavior, so I'm inclined to reject and to propose that you fork the project to make a new one. Not trying to discourage you at all, but I just don't have time to maintain this project anymore, and it takes me a long time just to grok huge changes like this one.

In principle I am sincerely hoping that local-npm prompts others to write their own alternative npm servers that hopefully can do a lot more than local-npm can... but for now I'm trying to mostly just keep it as-is as a reference implementation. If you think I'm wrong for rejecting, please reach out and I can try to take a second look, but as is it just looks like a huge change, and I'm not 100% sure what problem it's solving.

And @wmhilton replied: @nolanlawson I totally understand! I would have prefered not to fork simply because you've put an incredible amount of work into this and I want to contribute back.

In principle I am sincerely hoping that local-npm prompts others to write their own alternative npm servers that hopefully can do a lot more than local-npm can...

In that sense, I think you've succeeded! In my mind I have a significantly different direction I want to take the project, and perhaps "local" npm would be a misnomer for what I have in mind anyway.

Thank you again for making this! It was a huge boon to my little startup company to be able to cache every download. I had my computer, my partner's computer, and all our servers pointing to a local-npm instance for the node registry. That way if an install worked once, for one of us, I could rest sure that that package was cached and would be installable on all our servers into the future, regardless of whether npmjs.org was having regional 502 outages (which frustrated me) or rogue users liberated their modules deleted my dependencies. There are other efforts going on to make immutable registries, and I think my fork will probably steer in that direction.

billiegoose commented 7 years ago

TODO: Incorporate some of that last comment into the "Rationale" or "Motivation" section of the README.