Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Weird routing behavior deviating from Express #197

Closed
dimdenGD opened this issue Nov 11, 2023 · 3 comments
Closed

Weird routing behavior deviating from Express #197

dimdenGD opened this issue Nov 11, 2023 · 3 comments

Comments

@dimdenGD
Copy link

Consider this code:

import express from 'express';
import HyperExpress from 'hyper-express';
import LiveDirectory from 'live-directory';
import path from 'path';
import mime from 'mime-types';

const USE_HYPER_EXPRESS = false;

const app = USE_HYPER_EXPRESS ? new HyperExpress.Server() : express();

const assets = new LiveDirectory('public', {
    static: process.env.NODE_ENV === 'production',
    cache: {
        max_file_count: 100,
        max_file_size: 1024 * 1024 * 2.5,
    }
});

function serveStaticFile(req, res, assets) {
    let p = req.path;
    if(!req.path.includes('.')) {
        p = path.join(req.path, 'index.html').replaceAll('\\', '/'); // for some reason, live-directory doesn't seem to support Windows paths
    }
    const asset = assets.get(p);
    if(!asset) {
        return res.status(404).send(`Static file '${p}' not found`);
    }
    
    res.header('Content-Type', mime.lookup(p) ?? 'application/octet-stream');

    if (asset.cached) {
        return res.send(asset.content);
    } else {
        const readable = asset.stream();
        return readable.pipe(res);
    }
}

app.get('/', (req, res) => {
    res.send('Hello World!');
});

app.get('/:name', (req, res, next) => {
    if(req.params.name === 'cat.jpg') {
        return next();
    }
    res.send(`Hello ${req.params.name}!`);
});

app.use((req, res) => {
    serveStaticFile(req, res, assets);
});

// another inconsistency with express, would be nice to support callbacks when they're supplied
if(USE_HYPER_EXPRESS) {
    app.listen(3001)
        .then(() => {
            console.log('Listening on port 3001');
        }).catch((err) => {
            console.error(err);
        });
} else {
    app.listen(3001, () => {
        console.log('Listening on port 3001');
    });
}

In this code, you can set USE_HYPER_EXPRESS to either true or false whether if you want to use hyper-express or not.

There's an endpoint /:name that behaves differently.

If you use express:

  • If you go to localhost:3001/test it will return Hello test!
  • If you go to localhost:3001/cat.jpg it will return the cat jpg inside public folder

If you use hyper-express:

  • If you go to localhost:3001/test it will return Static file '/test/index.html' not found, completely ignoring .get('/:name') endpoint
  • If you go to localhost:3001/cat.jpg it will return the cat jpg inside public folder

Here's zip with code and files:
hyperexpress.zip

@kartikk221
Copy link
Owner

The implementation for chained routes from #198 should potentially resolve this issue and supporting callbacks would certainly reduce the incompatibility gap.

Are you aware of any other methods which hyper-express currently only offers as Promises but should support callbacks for better compatibility with Express?

@dimdenGD
Copy link
Author

I don't know of any other methods like that

@kartikk221
Copy link
Owner

So changing the routing behavior in the current version would be a breaking change for the current version and users. So the routing behavior consistency will be implemented in the next major v7.0 update which is currently in the works. I'll be sure to follow up once that is released to ensure all concerns are handled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants