Skip to content

Conversation

@aaron-blondeau-dose
Copy link

I have been unable to find a way to add arena to an existing express app while also implementing authentication. The basePath parameter conflicts with any path other than "/" provided to express' use method. Allowing users to provide a new parameter called "mountPath" can alleviate this issue and allow configs like the following to work:

const arena = Arena({
    queues
}, {
    basePath: '/admin/',
    mountPath: '/',
    disableListen: true
})

app.use('/admin', basicAuth('foo', 'bar'), arena)

where basicAuth = https://github.com/expressjs/basic-auth-connect

@javorosas
Copy link

I'm looking forward to this


app.use(app.locals.appBasePath, express.static(path.join(__dirname, 'public')));
app.use(app.locals.appBasePath, routes);
app.locals.mountPath = listenOpts.mountPath || app.locals.basePath;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless I'm reading this incorrectly, you intended for this to be listenOpts.mountPath || app.locals.appBasePath;, right?

@dbronin
Copy link

dbronin commented Apr 30, 2019

Any updates on this?

@bogdan
Copy link
Contributor

bogdan commented Nov 18, 2019

I have no idea why it is not merged yet.
However, here is a workaround at the moment:

const arena = Arena({
    queues
}, {
    basePath: '/',
    disableListen: true
})

// powerhacking
arena.locals.appBasePath = "/admin"

app.use('/admin', basicAuth('foo', 'bar'), arena)

Copy link
Member

@skeggse skeggse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This has been a long-requested feature. I'd like to move this along, but still don't have a ton of time to review. I don't immediately understand the distinction between the appBasePath and the mountPath. Along those lines, could we get some documentation in the readme describing how the new option functions and how it differs from the old?

@skeggse
Copy link
Member

skeggse commented Aug 6, 2020

Merging #182 into this.

@orchard-insights orchard-insights closed this by deleting the head repository Dec 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants