Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions webdev/lib/src/command/configuration.dart
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ const disableDdsFlag = 'disable-dds';
const enableExperimentOption = 'enable-experiment';
const canaryFeaturesFlag = 'canary';
const offlineFlag = 'offline';
const spaFallbackFlag = 'spa-fallback';

ReloadConfiguration _parseReloadConfiguration(ArgResults argResults) {
var auto = argResults.options.contains(autoOption)
Expand Down Expand Up @@ -109,6 +110,7 @@ class Configuration {
final List<String>? _experiments;
final bool? _canaryFeatures;
final bool? _offline;
final bool? _spaFallback;

Configuration({
bool? autoRun,
Expand Down Expand Up @@ -136,6 +138,7 @@ class Configuration {
List<String>? experiments,
bool? canaryFeatures,
bool? offline,
bool? spaFallback,
}) : _autoRun = autoRun,
_chromeDebugPort = chromeDebugPort,
_debugExtension = debugExtension,
Expand All @@ -158,7 +161,8 @@ class Configuration {
_nullSafety = nullSafety,
_experiments = experiments,
_canaryFeatures = canaryFeatures,
_offline = offline {
_offline = offline,
_spaFallback = spaFallback {
_validateConfiguration();
}

Expand Down Expand Up @@ -234,7 +238,8 @@ class Configuration {
nullSafety: other._nullSafety ?? _nullSafety,
experiments: other._experiments ?? _experiments,
canaryFeatures: other._canaryFeatures ?? _canaryFeatures,
offline: other._offline ?? _offline);
offline: other._offline ?? _offline,
spaFallback: other._spaFallback ?? _spaFallback);

factory Configuration.noInjectedClientDefaults() =>
Configuration(autoRun: false, debug: false, debugExtension: false);
Expand Down Expand Up @@ -291,6 +296,8 @@ class Configuration {

bool get offline => _offline ?? false;

bool get spaFallback => _spaFallback ?? false;

/// Returns a new configuration with values updated from the parsed args.
static Configuration fromArgs(ArgResults? argResults,
{Configuration? defaultConfiguration}) {
Expand Down Expand Up @@ -419,6 +426,10 @@ class Configuration {
? argResults[offlineFlag] as bool?
: defaultConfiguration.verbose;

final spaFallback = argResults.options.contains(spaFallbackFlag)
? argResults[spaFallbackFlag] as bool?
: defaultConfiguration.spaFallback;

return Configuration(
autoRun: defaultConfiguration.autoRun,
chromeDebugPort: chromeDebugPort,
Expand All @@ -445,6 +456,7 @@ class Configuration {
experiments: experiments,
canaryFeatures: canaryFeatures,
offline: offline,
spaFallback: spaFallback,
);
}
}
Expand Down
4 changes: 4 additions & 0 deletions webdev/lib/src/command/serve_command.dart
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ refresh: Performs a full page refresh.
..addFlag(logRequestsFlag,
negatable: false,
help: 'Enables logging for each request to the server.')
..addFlag('spa-fallback',
Copy link
Member

Choose a reason for hiding this comment

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

Any interest on more thoroughly documenting this param here? Is there a better place to describe this? (it seems that lots of parameters are missing)

negatable: false,
help: 'Serves index.html for any 404 from a non-asset request. '
'Useful for single-page applications with client-side routing.')
..addOption(tlsCertChainFlag,
help:
'The file location to a TLS Certificate to create an HTTPs server.\n'
Expand Down
37 changes: 37 additions & 0 deletions webdev/lib/src/serve/webdev_server.dart
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,43 @@ class WebDevServer {
cascade = cascade.add(assetHandler);
}

if (options.configuration.spaFallback) {
FutureOr<Response> spaFallbackHandler(Request request) async {
Copy link
Member

Choose a reason for hiding this comment

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

Instead of defining this inline, maybe this could be defined in a separate file, similar to handlers/favicon_handler.dart?

That could make it a little bit easier to add a test for!

final hasExt = request.url.pathSegments.isNotEmpty &&
request.url.pathSegments.last.contains('.');
Comment on lines +200 to +201
Copy link
Member

Choose a reason for hiding this comment

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

What do we see here if the request.url ends in /? is the last pathSegment an empty string, or is it the name of the last segment before the /?

if (request.method != 'GET' || hasExt) {
return Response.notFound('Not Found');
}

final indexUri =
request.requestedUri.replace(path: 'index.html', query: '');
Copy link
Member

Choose a reason for hiding this comment

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

I'm surprised that index.html is not configurable in the serve command or elsewhere, wow. I'd move index.html to a const at the top of the file though.


final cleanHeaders = Map.of(request.headers)
..remove('if-none-match')
..remove('if-modified-since');

final proxiedReq = Request(
'GET',
indexUri,
headers: cleanHeaders,
context: request.context,
protocolVersion: request.protocolVersion,
);

final resp = await assetHandler(proxiedReq);

if (resp.statusCode != 200 && resp.statusCode != 304) {
return Response.notFound('Not Found');
}
return resp.change(headers: {
...resp.headers,
'content-type': 'text/html; charset=utf-8',
});
}

cascade = cascade.add(spaFallbackHandler);
}

final hostname = options.configuration.hostname;
final tlsCertChain = options.configuration.tlsCertChain ?? '';
final tlsCertKey = options.configuration.tlsCertKey ?? '';
Expand Down