fix(#161): use() accepts array

[jimisaacs]

This should allow use() to behave more like express.use

[changeset-bot[bot]]

⚠️ No Changeset found

Latest commit: dfd24aae8d7536f5f41b55ea22101cf6dbd1665a

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[codecov[bot]]

Codecov Report

Merging #162 (dfd24aa) into master (89196b1) will not change coverage. The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #162   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines           86        87    +1     
=========================================
+ Hits            86        87    +1     

Impacted Files	Coverage Δ
src/index.js	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 89196b1...dfd24aa. Read the comment docs.

[hoangvvo]

Could you let me know some use cases around this? Is there any libraries that require this pattern?

Otherwise, I think this can simply perform a .map to register the handlers:

["/foo", "/bar"].map((base) => nc.use(base, fn))

[jimisaacs]

The use case is for an internal library written for express to be used with next. I'd like to use next without modifying that library and without using a next custom server approach, and I was looking to this library to achieve that, as I read "drop-in replacement".

[hoangvvo]

I would be more than happy to merge this. There is just a few nits!

[jimisaacs]

@hoangvvo Ok sorry for the long long delay, but I've reworked this a bit. Should be much much simpler. Only handling one case here. This one:

app.use([middleware1, middleware2]);

Which a lot of middlewares expect.

[hoangvvo]

@hoangvvo Ok sorry for the long long delay, but I've reworked this a bit. Should be much much simpler. Only handling one case here. This one:

app.use([middleware1, middleware2]);

Which a lot of middlewares expect.

Thank you! So in this, the array middleware pattern only works if it is the first argument. Is it how it works in express.js too? Would there be a use case for having a base then a middeware array?

const mArr = [m1,m2,m3];
handler.use("/foo", mArr);

[jimisaacs]

@hoangvvo if you look at the express code, they support a lot of crazy things. All sorts of positions and levels of arrays. If you look at their types, it's similar, but not quite as open ended as their JavaScript. As an example, in their JavaScript, they use a recursive while loop lookup, just to find the first string argument.

This is the reason I kept it simple. Because the most common use case in the express world, is the one I'm adding support for. The most common use case for middleware is to be added to the app without a route, though yes, they support arrays everywhere middleware is taken, at all levels.

If you are interested in supporting exactly what express does, I can look into that, but it would most likely be just copying a lot of their code. What are you thinking?

[hoangvvo]

@hoangvvo if you look at the express code, they support a lot of crazy things. All sorts of positions and levels of arrays. If you look at their types, it's similar, but not quite as open ended as their JavaScript. As an example, in their JavaScript, they use a recursive while loop lookup, just to find the first string argument.

This is the reason I kept it simple. Because the most common use case in the express world, is the one I'm adding support for. The most common use case for middleware is to be added to the app without a route, though yes, they support arrays everywhere middleware is taken, at all levels.

If you are interested in supporting exactly what express does, I can look into that, but it would most likely be just copying a lot of their code. What are you thinking?

I saw your previous implementation support this pattern of having array in multiple places, but this implementation is obviously so much cleaner. I am okay with just supporting only the first argument. However, if we go with the other one, another hacky solution is to use Array.flat to just flatten out the array middlewares and process as expected.

[jimisaacs]

That's what express does, but along with some additional processing. Just want to get to know your thoughts here.

[hoangvvo]

That's what express does, but along with some additional processing. Just want to get to know your thoughts here.

If we would add support to this, I think it's better to support all positions.