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

Switch to using a global property for domain sharding #284

Merged
merged 2 commits into from
Dec 16, 2024

Conversation

benjervis
Copy link
Contributor

Originally domain sharding was set up to work by looking for the presence of some specified cookie to decide whether to shard or not, as this is what the CFE SSR server does already.

However that's a little less easy to use, and other products would prefer to use a key on the window object instead.

Now, the runtime looks for a boolean value in window.__ATLASPACK_ENABLE_DOMAIN_SHARDS as its switch.

SSR servers can inject an inline script tag at the top of the document if sharding is required.

<script>
  window.__ATLASPACK_ENABLE_DOMAIN_SHARDS = true;
</script>

Or, if cookie based logic is still preferred

<script>
  window.__ATLASPACK_ENABLE_DOMAIN_SHARDS = document.cookie.indexOf('existing.cookie.name') !== -1;
</script>

@benjervis benjervis requested a review from a team December 13, 2024 08:13
@@ -130,7 +109,7 @@ describe('bundle-url-shards helper', () => {
const code = await overlayFS.readFile(mainBundle.filePath, 'utf-8');
assert.ok(
code.includes(
`require("449af90f11ccd363").getShardedBundleURL('b.8575baaf.js', '${testingCookieName}', document.cookie, ${maxShards}) + "b.8575baaf.js"`,
`require("e73f1ec3075dec82").getShardedBundleURL('b.8575baaf.js', Boolean(window.__ATLASPACK_ENABLE_DOMAIN_SHARDS), ${maxShards}) + "b.8575baaf.js"`,
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: If you inline window.__ATLASPACK_ENABLE_DOMAIN_SHARDS into the helper, bundle size will be smaller (the global var also can't be minified).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As in do the check inside the function? I wanted to avoid having to pass the whole window object through, mostly for testing purposes

Copy link
Contributor

@JakeLane JakeLane Dec 15, 2024

Choose a reason for hiding this comment

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

Yeah inside the function you could check globalThis.__ATLASPACK_ENABLE_DOMAIN_SHARDS (rather than passing through the window object). Then mock the globalThis object in test.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I've pushed a change trying that out

@benjervis benjervis force-pushed the allow-window-property-for-sharding branch from 7e5de5f to 22f375d Compare December 16, 2024 00:01
@benjervis benjervis force-pushed the allow-window-property-for-sharding branch from 22f375d to edc1d34 Compare December 16, 2024 00:01
@benjervis benjervis enabled auto-merge (squash) December 16, 2024 00:11
@benjervis benjervis merged commit 6d1bb40 into main Dec 16, 2024
17 checks passed
@benjervis benjervis deleted the allow-window-property-for-sharding branch December 16, 2024 00:15
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.

3 participants