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

feat: add web support #62

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

ariefwijaya
Copy link
Contributor

resolved #61

Please let me know if you have another approach. I split the file to make it easier to read and backwards compatible without touching existing main scripts smart_network_asset_loader.dart

@ariefwijaya
Copy link
Contributor Author

@aissat Need your help to review. Thanks

@bw-flagship
Copy link
Collaborator

@ariefwijaya Thanks that you submitted this PR, the motivation makes sense!

What I don't like is that all the logic is duplicated now. If we would make an update to the smart_network_asset_loader, one would need to remember to copy-paste the update to the other file.

I'm not aware if there is a state of the art solution for this, but I would propose the following:

Create a new file called smart_network_asset_loader_base.dart.
It is as the current smart_network_asset_loader, but it is abstract and the the platform-dependent part extracted into an (abstract) method.

smart_network_asset_loader and smart_network_asset_loader_web both inherit from SmartNetworkAssetLoaderBase and implement the native part accordingly.

Wdyt?

@ariefwijaya
Copy link
Contributor Author

@bw-flagship Thank you, this is a great idea and as a best practice, it will help us upgrade to other platforms more easily and consistently. I'll refactor the code and update my PR later. (a bit busy now). Anyone who wants to help is welcome

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.

[Feature Request] Add web support
2 participants