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

Rework AssetHandler #870

Merged
merged 6 commits into from
Jan 16, 2020
Merged

Rework AssetHandler #870

merged 6 commits into from
Jan 16, 2020

Conversation

grouma
Copy link
Member

@grouma grouma commented Jan 15, 2020

  • Rename AssetHandler to AssetReader and move to src/readers
  • Update AssetReader API to make clear what resources need to be made available to DWDS
  • Rename BuildRunnerAssetHandler to ProxyServerAssetReader
  • Break out ProxyServerAssetReader into proxy_server_asset_reader.dart and move to src/readers
    • Note in the future we will have FrontendServerAssetReader with similar functionality
  • Move readAssetOrNull logic into ProxyServerAssetReader as it will likely not apply to the FrontendServerAssetReader
  • Update webdev to adhere to the new API requirements

Towards #865

import 'dart:async';

/// A handler for Dart sources and related source maps.
abstract class AssetHandler {
Copy link
Contributor

@jakemac53 jakemac53 Jan 15, 2020

Choose a reason for hiding this comment

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

should we rename this to AssetReader or something since it no longer expose a Handler?

Copy link
Member Author

Choose a reason for hiding this comment

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

Was initially planning on doing that but thought you wouldn't like it, so I left it as is. Should have chatted with you first. I'll clean this up.

import 'asset_reader.dart';

/// A reader for resources provided by build runner.
class BuildRunnerAssetReader implements AssetReader {
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider ProxyServerAssetReader or something along those lines? It doesn't really have anything build-runner specific it just delegates the reads to a proxy server.

@grouma grouma merged commit 0ebfb41 into master Jan 16, 2020
@grouma grouma deleted the asset-handler branch January 16, 2020 18:55
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

Successfully merging this pull request may close these issues.

2 participants