fix: assign domain after cookie storage options are given

[dnshi]

Summary

Because this domain value is needed while initializing metadata storage, assign options.domain after cookie storage options are given.

Checklist

• Does your PR title have the correct title format?
• Does your PR have a breaking change?: No

[kevinpagtakhan]

im curious how this impacts sdk usages that did not use defer init. would they have existing cookie without the domain suffix? if we move it up to this line, then would it look for cookie with the domain prefix, which it may not find?

[dnshi]

It is really interesting that we actually have two cookie names: with domain and without domain.

And it does not defer init

[dnshi]

When initializing the metadata storage without domain, all cookies saved by the metadata storage does not include the domain suffix. After domain is assigned to this.options.domain, SDK looks for cookie but cannot find it, and then a new cookies with domain suffix is created. I think this is the flow.

[kevinpagtakhan]

Hmm not sure about that, but isn't metadata storage only instantiated once for the most part? then does this mean even if this.options.domain changes, it would still use the initial storage key?

[dnshi]

isn't metadata storage only instantiated once for the most part?

If deferInitialization is not set, then it is only one metadata storage instance. But for defer init, a new metadata storage instance is initialized with the new domain value.

then does this mean even if this.options.domain changes, it would still use the initial storage key?

If this.options.domain changes, the storage key is still the initial one.

Here we have two options:

1. Move the this.options.domain = this.cookieStorage.options().domain; before initialize metadata storage, so the cookies include domain suffix.
2. Move the this.options.domain = this.cookieStorage.options().domain; after the defer init check, so the metadata storage is initialized again, the storage key is still the same.

I pick the first one option, but I am thinking if you have any other concern.

[dnshi]

From setDomain method, the this.options.domain is updated immediately.

Amplitude-JavaScript/src/amplitude-client.js

Lines 862 to 880 in 52abaf0

	AmplitudeClient.prototype.setDomain = function setDomain(domain) {
	if (this._shouldDeferCall()) {
	return this._q.push(['setDomain'].concat(Array.prototype.slice.call(arguments, 0)));
	}
	if (!utils.validateInput(domain, 'domain', 'string')) {
	return;
	}
	try {
	this.cookieStorage.options({
	expirationDays: this.options.cookieExpiration,
	secure: this.options.secureCookie,
	domain: domain,
	sameSite: this.options.sameSiteCookie,
	});
	this.options.domain = this.cookieStorage.options().domain;
	_loadCookieData(this);
	_saveCookieData(this);

[kevinpagtakhan]

Yeah my concern is what I mentioned in my first comment. Like if they don't defer init, which is default behavior, and client devices have an existing cookie key with no suffix. If we go with option 1, then it will not find the existing cookies right? If it does not find existing cookies, then it will generate new session ids and device ids and potentially disassociate events that would have been for the same amplitude ID, I'm thinking.

[dnshi]

Right. I think it is a valid concern. I will move to option 2.