pouchdb / pouchdb-fauxton

Fork of CouchDB Fauxton for PouchDB Server
Apache License 2.0
6 stars 7 forks source link

Serve Fauxton from relative path. #5

Closed MichaelJCole closed 7 years ago

MichaelJCole commented 9 years ago

Hi, this PR takes @thesli's comments and suggestions and packages them into a PR.

You can test this PR like this:

1) Clone couchdb-fauxton 2) grunt release to build the changes. 3) put together a test server:

package.json

{
  "name": "test",
  "version": "1.0.0",
  "description": "",
  "main": "test.js",
  "scripts": {
    "test": "echo \"Error: no test specified\" && exit 1"
  },
  "author": "",
  "license": "ISC",
  "dependencies": {
    "express": "^4.9.8",
    "express-pouchdb": "^0.7.1",
    "morgan": "^1.4.0",
    "pouchdb": "^3.0.6"
  }
}

Test.js

var express = require('express')
  , app     = express()
  , PouchDB = require('pouchdb')
  , logger  = require('morgan')
  ;

app.use(logger("tiny"));
app.get('/', function(req, res, next) { 
  res.send('Welcome Traveler!  <a href="/express-pouchdb/_utils">Fauxton?</a>');
});
app.use('/custom', require('express-pouchdb')(PouchDB));

app.listen(3000);

To run the server with new Fauxton (assuming test and couchdb-fauxton are in same dir)

npm install
cd node_modules/express-pouchdb
mv fauxton fauxton.old
ln -s ../../../couchdb-fauxton/dist/release fauxton
node test.js

This addresses several issues identified and a couple extra. From my testing, these issues remain, but none of them are show stoppers. Compared to not working, 90% working is much better!

tomsun commented 9 years ago

@MichaelJCole 27784fe is a bit confusing (hardcoded path specific to express-pouchdb), maybe squash it with the following commit (which basically un-does it), for better readability?

Btw, @nolanlawson hinted in #6 a couple of days ago that he would prefer this kind of change to be directed towards Fauxton proper

MichaelJCole commented 9 years ago

@tomsun Yeah, I agree the PR could be cleaner. Sorry about that. Re: upstream. I contacted the Fauxton devs via IRC and their bug tracker. They were supportive, but not prioritized to fix it in Fauxton.

There are some incompatible change around the links to docs as well.

nolanlawson commented 9 years ago

Yeah, I tend to think this is something that should be fixed upstream in Fauxton. This fork should really just be for PouchDB-specific stuff like branding and colors. It would be a shame if Fauxton itself never supported it.

That being said, if it's not high-priority for Fauxton, then I think this is worth merging. @marten-de-vries ?

marten-de-vries commented 9 years ago

I would really like our changes to Fauxton to just be theming and a settings file since that would keep upgrading our Fauxton from upstream easier. The current version is quite out of date (e.g. deleting databases is broken IIRC).

If it's impossible to get this into upstream Fauxton though, I'm ok with merging it. The important thing is to move this forward.

garth commented 8 years ago

Is there currently a way to get fauxton with pouchdb-express working under /db without this patch?

marten-de-vries commented 8 years ago

@garth Seems like it's not. (I just gave it a try).

garth commented 8 years ago

@marten-de-vries perhaps another option might be to do some route/url rewriting in express so that pouch is always running at the root?

marten-de-vries commented 8 years ago

@garth Possible, but this PR is better. Maybe we should just merge it considering no one seems to want to take it upstream.

garth commented 8 years ago

@marten-de-vries that would be great. Maybe fauxton could be updated to the latest release at the same time.

jvgreenaway commented 7 years ago

This is still a bug in express-pouchdb

nolanlawson commented 7 years ago

I think we should work on rebasing on the latest Fauxton since I believe they've fixed this issue upstream.

nolanlawson commented 7 years ago

Hi, this repo has been completely rewritten, and the change would now go to http://github.com/apache/couchdb-fauxton . Please file it there if/when you get a chance. :smiley:

rjcorwin commented 7 years ago

@nolanlawson This is bummer to not have Fauxton working with the express-pouchdb application servers. Should we file an issue in apache/couchdb-fauxton? A quick search doesn't pull up anything on folk's needing support for relative paths in fauxton. https://github.com/apache/couchdb-fauxton/search?q=relative+path&type=Issues&utf8=%E2%9C%93

Perhaps it's a non-issue now and we need to get the fauxton code over here? I'm not familiar with how it's all strapped together.