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

Add domain sharding to js runtime #268

Merged
merged 26 commits into from
Dec 12, 2024
Merged

Conversation

benjervis
Copy link
Contributor

In order to support domain sharding for HTTP1.1 users, we need to apply shards in two places. Firstly, in the generated HTML, which is done at request time by the SSR server (as we need to know whether this is a 1.1 request or not). Secondly, when requesting bundles from within other bundles we need to shard those requests as well, and not just rely on the parent shard.

To support this, I've made some changes to the JS Runtime, adding in a sharded version of helpers/bundle-url which is activated if the domain sharding config is provided in a build.

"@atlaspack/runtime-js": {
  "domainSharding": {
    "maxShards": 5,
    "cookieName": "should.shard.domains"
  }
}

bundle-url-shards started out as a copy of bundle-url, but has been refactored a bit to make the more complicated logic easier to work with. That number of functions also became a little tricky to track, so the file is now a TypeScript file which will be compiled in the client build.

There was a todo in here to migrate getOrigin to just using the URL constructor once we didn't need to support IE 11 anymore, which we don't, so I've made that change and brought it across to bundle-url also.

Lastly there are a bunch of tests to make sure the sharding logic does what it's meant to, and that the runtime generates the call with the right arguments.

@benjervis benjervis requested a review from a team December 9, 2024 21:42
}

function getDomainShardIndex(str: string, maxShards: number) {
let shard = str.split('').reduce((a, b) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: a conventional for loop would be faster

@benjervis benjervis force-pushed the add-domain-sharding-to-js-runtime branch 2 times, most recently from ec74432 to 71b04c2 Compare December 12, 2024 05:56
@benjervis benjervis force-pushed the add-domain-sharding-to-js-runtime branch from 71b04c2 to 89d074e Compare December 12, 2024 06:16
Copy link
Contributor

@mattcompiles mattcompiles left a comment

Choose a reason for hiding this comment

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

Nice one bud!

@mattcompiles mattcompiles merged commit 6edef35 into main Dec 12, 2024
17 checks passed
@mattcompiles mattcompiles deleted the add-domain-sharding-to-js-runtime branch December 12, 2024 09:42
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.

5 participants