Open nightpool opened 5 years ago
here's a simple example case:
const app = express();
const user = express.Router();
user.get('/', (request, response) => {
const [username] = request.subdomains;
response.send(`homepage for ${username}!`);
});
app.use(subdomain('*', user));
app.get('/', (request, response) => {
response.send('this is the root domain!');
});
Any thoughts on this? Definitely open to feedback here but if you don't want a subdomain at all, why not just use .use
? I think this makes the most sense and I needed this change to make my app work at all.
Hey, just wanted to ping this again and ask if you've had a chance to take a look, or thought about whether this change makes sense!
@nightpool Sorry, I've not been active in this repo for a while. I think you might be right, looking at the logic again we're defaulting match=true
and only set it to false
if we don't match. Could you submit a PR with a failing test first to make sure we're on the same page and we can take it from there?
Currently, '*' matches both https://user.example.com and https://example.com. However, this doesn't make a ton of sense, and it makes it really hard to represent a bunch of common use-cases.
This is probably a breaking change, and it should probably get a version bump. I'm also open to suggestions to make it a non-breaking change (like adding an option flag to control this behavior) or a less-breaking change (like using a different symbol (
+
) for wildcards that require the presence of a domain and wildcards that don't). It's your repository, tell me what you think is best!tests should also be added around this behavior. I didn't look super closely but i don't think this breaks any existing tests.