SPA setting: ignore files with extensions

[jackfranklin]

When running our app we do want requests like /123/ to be converted to
/#/123, however things like /style.css or /foo.png should be
ignored. This commit changes the behaviour for the spa setting so it
only applies to requests without an extension.

This is a breaking change - I did also think if you were not so keen on this we could introduce a new configuration setting / CLI flag?

[tapio]

This sounds reasonable, but I don't use the SPA feature myself so I would like to hear some other comments. Also, I'm not keen on breaking changes...

[jackfranklin]

👍 understood. I appreciate not wanting a breaking change so I can definitely update this to need another flag.

[jackfranklin]

@tapio I have now updated to include --spa-ignore-assets / spaIgnoreAssets as an option which turns this feature on/off. It's off by default, and hence no breaking change.

[pavel]

This solution is not quite complete, what if someone would want /123.json to be converted to /#/123.json and /123.css to be kept as /123.css? This is highly coupled with one's project's architecture and one's needs. The perfect solution would be an URL rewrite plugin/module.

[jackfranklin]

@pavel I just went for the most straightforward approach to get it working. I guess you could have a spaWhitelist or spaBlacklist which could be a glob / regex of files to rewrite or not. Then the config starts to get very complicated though.

@tapio I don't know what you want to do here; I equally understand if you want to just not take this PR, in which case I'll maintain my version as a fork (as we're using it successfully at work), but if you would be open to either what I've done so far or something closer to @pavel's idea to allow users more control, just let me know.

[pavel]

@jackfranklin I do completely understand your point, I maintain my own fork with HTTP/2 support. But, please, understand that this is a general tool, it should work for everyone not just me and you. It is up to @tapio to decide what to do with this PR. Don't think I'm being bossy here or not like what you did, just want to make this tool as helpful as possible.

[jackfranklin]

@pavel I'm not saying it shouldn't work for everyone :) Just trying to find the best approach that doesn't make the config really complex.

I think maybe we could actually do both:

• keep -spa-ignore-assets as is if the dumb "just don't rewrite requests for .ext" is enough
• keep -spa as is for those who want to rewrite every request
• allow some form of URL rewriter to be passed for those who want full control: --router router.js or something, which will be required and run by live-server.

@tapio what do you think? If the above sounds too complex I understand and I'm happy for the PR to be closed. It's your module :)

[jackfranklin]

@pavel I've now updated this PR so spaIgnoreAssets can be given as a function that should return true when the asset should be rewritten.

@tapio please let me know if you would potentially merge this. If not, no worries, but I'll then maintain a fork (we need this at work), and endeavour to keep it up to date. I'd love any thoughts or feedback.

I think with this latest change it works for everyone:

• non breaking change by default
• flag for easy config which just ignores any requests with an asset
• can pass a function if you really need a lot of control.

[pavel]

I wouldn't rush this, as I was working on --rewrite option implementation. I plan to get back to it, as soon as we resolve #137 in PR #146.

@jackfranklin If you really need this like right now, I would recommend adding this right now. We'll still be able to deprecate it as soon as proper --rewrite functionality arrives.

[jackfranklin]

@pavel I'm happy to contribute / work with you on the rewrite option. I didn't see #146, that looks awesome! We're using my fork at work but would love to get back onto stable, especially with 146 and the speed up it gives. I can rework this PR or open a new one that adds the rewrite option, if you want to share your thoughts on how it should work.

@tapio I've used this library for ages and I'm happy to contribute if you'd like more people to spread the load.

[pavel]

@jackfranklin Please, see #149.

[tapio]

Thanks for the contribution and discussion, but now that I have thought about this more I'm going to reject this PR for the following reasons:

• There are more generally useful ways to do the same.
• My original goal was to keep live-server lean and mean. I think --spa might already have been too specific special case and this basically adds a special case to that special case.
• Tomorrow somebody's going to be asking to add --spa-do-something-slightly-differently and we have to draw the line somewhere.

That being said, I still want to support this usage of live-server. @pavel's URL rewrite functionality is one way to do that, which can be supported by adding spa examples to readme / wiki.

However, I think a simple improvement of allowing specifying middleware files from cli would open up an awesome way to extend live-server while keeping the core code nice and tidy. We could even ship the separate middleware modules with live-server (instead of just sharing them in a wiki). So this PR's functionality would look something like live-server --middleware=spa-ignore-assets on the CLI. --spa would also be reimplemented like that and the --spa parameter deprecated.

The callback function version of the spa thingy can already be implemented by using the middleware node api parameter (with slight more code than in this PR, but in a much more generally useful way).

[jackfranklin]

@tapio that sounds good, I like the idea of middleware. I'm happy to work on this 👍

[tapio]

The basic version of middleware based spa-ignore-assets is in master branch. You can test it with --middleware=spa-ignore-assets.