feat(package_info_plus)!: Support multiple version.json locations

[AriasBros]

Description

Currently, the version.json file is fetched using a web specific API to get the base URI. Because the base URI obtained with this API is always the URL from the browser, this could lead that some setups where the Flutter app is deployed and served using different domains and paths could fail.

This PR changes that replacing the web specific API with a Flutter API to get the right URL used to load the assets of the application.

Related Issues

• Fix [Bug]: package_info_plus is not using the right URL to load the version.json file in web #2732

Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I titled the PR using Conventional Commits.
• I did not modify the CHANGELOG.md nor the plugin version in pubspec.yaml files.
• All existing and new tests are passing.
• The analyzer (flutter analyze) does not report any problems on my PR.

Breaking Change

Does your PR require plugin users to manually update their apps to accommodate your change?

• Yes, this is a breaking change (please indicate that with a ! in the title as explained in Conventional Commits).
• No, this is not a breaking change.

[miquelbeltran]

Thanks for the pr! I was experimenting with the same yesterday, it's kind of weird that the framework doesn't provide a direct way to obtain the base URLs. As pointed out, reading the base URL from the browser window is not exactly right in all cases.

My concern is if this would be wrong, given the case a user setups a different assets' URL than the entry point where the app is installed.

e.g.

_flutter.loader.loadEntrypoint({
    entrypointUrl: "https://www.app-domain-1.com/app/main.dart.js"
    onEntrypointLoaded: async function(engineInitializer) {
        let appRunner = await engineInitializer.initializeEngine({
            assetBase: "https://www.app-domain-2.com/myappcontent/"
        });
        appRunner.runApp();
    }
});

Where would we expect the version.json to be found? in domain 1 or 2?

So, we can have three URLs:

• The browser URL
• The entry point URL (where the Dart JS code is)
• The Assets URL

Another option is to use it as fallback, say, check if the version.json exists on assets base URL, if not, check on browser URL. This could be done together with also providing a custom param.

Allowing to do:

1. If a custom version.json path exists, use that.
2. If not, check in the assets' base URL.
3. If not, use browser URL.

[miquelbeltran]

Hi @AriasBros I am thinking on taking your commit 860b8ba and creatting a new PR that would support custom version.json url + fallback to browser baseUrl.

Do you mind if I get your commit and continue adding to that?

[AriasBros]

Hi @miquelbeltran,

Sorry the delay to answer you. I agree with your proposed solution, but these days my company changed my priorities and this one was down a little in the list, but I think I will reach this task today in a few hours.

I would like to send an update to this PR today or the monday.

If there is hurry to fix this issue just let me know and in that case of course you can use my commit, no problem with that.

[miquelbeltran]

Actually, I will just restore back to the original state of your PR, I will experiment on my own branch: miquelbeltran/2732

[miquelbeltran]

So far the implementation is looking good.

I've left a question regarding the custom baseUrl.

I'll do some tests when I got the time.

We also should add integration tests. Testing the custom version.json uri should be easy (like done here:

plus_plugins/packages/package_info_plus/package_info_plus/example/integration_test/package_info_plus_web_test.dart

Lines 83 to 100 in 095499d

	testWidgets(
	'Get package info using custom version json uri',
	(tester) async {
	when(client.get(customUri)).thenAnswer(
	(_) => Future.value(
	http.Response(jsonEncode(VERSION_JSON), 200),
	),
	);
	final versionMap = await plugin.getAll(customVersionJson: customUri);
	expect(versionMap.appName, VERSION_JSON['app_name']);
	expect(versionMap.version, VERSION_JSON['version']);
	expect(versionMap.buildNumber, VERSION_JSON['build_number']);
	expect(versionMap.packageName, VERSION_JSON['package_name']);
	expect(versionMap.buildSignature, VERSION_JSON['build_signature']);
	},
	);

). Testing the assets' manager may not be as easy without allowing to mock it from the outside, but it is something we can do at the end.

[AriasBros]

We also should add integration tests. Testing the custom version.json uri should be easy. Testing the assets' manager may not be as easy without allowing to mock it from the outside, but it is something we can do at the end.

Yes, it is the next thing I want to work on.

[miquelbeltran]

I'll mark the PR as breaking, just because the platform interface signature also changed, and I want to avoid version conflicts when pulling dependencies. Technically, this could pass as a non-breaking for package users, but I want to be on the safe side.

[miquelbeltran]

Thank you, David! I think this is good to go for now. Let me know if you still want to do some more changes.

[AriasBros]

Thank you, David! I think this is good to go for now. Let me know if you still want to do some more changes.

Thanks @miquelbeltran

From my side I don't see nothing else to do. Maybe update online documentation, but I am not sure if I should do it in this same PR or that follows other procedures. Just let me know about it, I am available for any change that this PR could need.

[AriasBros]

I'll mark the PR as breaking, just because the platform interface signature also changed, and I want to avoid version conflicts when pulling dependencies. Technically, this could pass as a non-breaking for package users, but I want to be on the safe side.

If it is necessary, I could split this PR in 2:

• One adding only the implementation about to use the AssetManager base URL, which it should not be a breaking change.
• And then other with the changes related to add the custom base URL parameter.

Anyway, in this last commit that I sent, I added a new parameter to the plugin constructor to allow testing the AssetManager. I understand that that constructor is a private API of the package, but well, maybe it could be considered a breaking change too.

[miquelbeltran]

Thanks for including a test with the AssetManager as well!

Having a breaking change is not an issue, it will simply go into a new major version when we generate a new release, which I prefer anyway. It may take a while until we release the package since we did a big release very recently, but this will be available to anyone who wants to try using a git ref dependency.

I'm specially interested if people in #2699 can give it a try.

Thanks again for the work! I will merge now, any further improvements we can do in a new PR.