Skip to content

Commit

Permalink
fix(v6.4.0 RC): conditionally remove SSELink from the bundle (#9401)
Browse files Browse the repository at this point in the history
Follow up to #9009 and #9205.

<table>
<tr><th>stable (v6.3.3)</th><th>v6.4.0 RC</th><th>% increase</th></tr>
<tr><td>

- Rendered: 1.03MB (100.00%)
- Gzip: 338.99KB
- Brotli: 290.87KB

</td><td>

- Rendered: 1.46MB (100.00%)
- Gzip: 461.74KB
- Brotli: 394.16KB

</td><td>

- Rendered: 41.75%
- Gzip: 36.21%
- Brotli: 35.51%

</td></tr>
</table>

I'm using
[rollup-plugin-visualizer](https://www.npmjs.com/package/rollup-plugin-visualizer)
on a newly-created Redwood app to get these numbers and treemaps. Here's
links to the full treemaps:
-
[stable](https://github.com/redwoodjs/redwood/assets/32992335/41cf4204-10de-4b69-a07a-7d6b980ead31)
- [v6.4.0
RC](https://github.com/redwoodjs/redwood/assets/32992335/f1a6a5bf-a260-45cb-8c97-f68b4fe0730d)

We don't have a history of bundle size metrics, so it's hard to tell
what the magnitude of this change is. But obviously in general, an
increase isn't good, especially if it's for a feature that isn't used
across the board.

One solution was mentioned in this thread:
#9009 (comment).
It was to have users import the link, that way if they didn't, Vite
wouldn't bundle it. The pushback there was more config for the user.

I paired with @Josh-Walker-GM on a solution that doesn't involve any
tradeoffs, except that it was work for us: we use Vite to remove the
file from the bundle if realtime isn't configured. No bundle size
increase if you're not using realtime, no config if you are!

Note that we need to watch out for
#9342 since it removes the
option from the toml file that this solution uses.

This was the main issue blocking v6.4.0, so now that it's resolved,
we're ready to ship next week.

**Update**

This PR was failing the serve smoke test consistently. At first I
thought it was because I didn't add the same logic to our webpack
config, so I went ahead and did that but it was still failing. After a
few dead ends, it turns out the logic I naively copied from the router
to conditionally render the splash page has had a bug all along:

https://github.com/redwoodjs/redwood/blob/699723910904d243ab153833b2d1a8e7494fcf40/packages/router/src/router.tsx#L120

You can see it easily by building and serving a Redwood app that doesn't
have a home page:

<img width="1186" alt="image"
src="https://github.com/redwoodjs/redwood/assets/32992335/b4ee1369-af45-4887-aacf-3217212bdc68">

This is because this line is transpiled into:

```js
var _splashPage = requireSplashPage();
// ...
if (shouldShowSplash && typeof _splashPage.SplashPage !== "undefined") {
```

And you can't get away with `undefined.SplashPage`. The reason we didn't
see it before is that hardly any user run `yarn rw serve` without a home
page.

Fixed by changing the module from a default `null` export to an empty
object export:
591ee5a.
  • Loading branch information
jtoar committed Nov 13, 2023
1 parent 56cdefa commit b3ed80d
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 47 deletions.
5 changes: 5 additions & 0 deletions packages/core/config/webpack.common.js
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,11 @@ module.exports = (webpackEnv) => {
filename: 'static/media/[name].[contenthash:8][ext]',
},
},
// (8)
!redwoodConfig.experimental.realtime.enabled && {
test: require.resolve('@redwoodjs/web/dist/apollo/sseLink'),
use: require.resolve('null-loader'),
},
].filter(Boolean),
},
],
Expand Down
3 changes: 3 additions & 0 deletions packages/project-config/src/__tests__/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ describe('getConfig', () => {
"enabled": false,
"wrapApi": true,
},
"realtime": {
"enabled": false,
},
"studio": {
"graphiql": {
"authImpersonation": {
Expand Down
6 changes: 6 additions & 0 deletions packages/project-config/src/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,9 @@ export interface Config {
plugins: CLIPlugin[]
}
useSDLCodeGenForGraphQLTypes: boolean
realtime: {
enabled: boolean
}
}
}

Expand Down Expand Up @@ -181,6 +184,9 @@ const DEFAULT_CONFIG: Config = {
],
},
useSDLCodeGenForGraphQLTypes: false,
realtime: {
enabled: false,
},
},
}

Expand Down
6 changes: 6 additions & 0 deletions packages/vite/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,12 @@ export default function redwoodPluginVite(): PluginOption[] {
id: /@redwoodjs\/router\/dist\/splash-page/,
},
]),
!rwConfig.experimental.realtime.enabled &&
removeFromBundle([
{
id: /@redwoodjs\/web\/dist\/apollo\/sseLink/,
},
]),
react({
babel: {
...getWebSideDefaultBabelConfig({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ export default function removeFromBundle(

// Currently configured for CJS only.
const EMPTY_MODULE = {
code: `module.exports = null`,
code: `module.exports = {}`,
}

export function excludeOnMatch(modulesToExclude: ModulesToExclude, id: string) {
Expand Down
37 changes: 20 additions & 17 deletions packages/web/src/apollo/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -190,23 +190,26 @@ const ApolloProviderWithFetchConfig: React.FunctionComponent<{

// Our terminating link needs to be smart enough to handle subscriptions, and if the GraphQL query
// is subscription it needs to use the SSELink (server sent events link).
const httpOrSSELink = apolloClient.split(
({ query }) => {
const definition = getMainDefinition(query)

return (
definition.kind === 'OperationDefinition' &&
definition.operation === 'subscription'
)
},
new SSELink({
url: uri,
auth: { authProviderType, tokenFn: getToken },
httpLinkConfig,
headers,
}),
httpLink
)
const httpOrSSELink =
typeof SSELink !== 'undefined'
? apolloClient.split(
({ query }) => {
const definition = getMainDefinition(query)

return (
definition.kind === 'OperationDefinition' &&
definition.operation === 'subscription'
)
},
new SSELink({
url: uri,
auth: { authProviderType, tokenFn: getToken },
httpLinkConfig,
headers,
}),
httpLink
)
: httpLink

// The order here is important. The last link *must* be a terminating link like HttpLink or SSELink.
const redwoodApolloLinks: RedwoodApolloLinks = [
Expand Down
65 changes: 36 additions & 29 deletions tasks/smoke-tests/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,43 +3,50 @@ import { expect, PlaywrightTestArgs } from '@playwright/test'
export async function smokeTest({ page }: PlaywrightTestArgs) {
await page.goto('/')

// Check that the blog posts are being loaded.
// Avoid checking titles because we edit them in other tests.
await page.textContent('text=Meh waistcoat succulents umami')
await page.textContent('text=Raclette shoreditch before they sold out lyft.')
await page.textContent(
'text=baby single- origin coffee kickstarter lo - fi paleo skateboard.'
)

// Check that the blog posts load. We're deliberately not checking their titles because we edit them in other tests.
await expect(
page.getByText(
'Meh waistcoat succulents umami asymmetrical, hoodie post-ironic paleo chillwave '
)
).toBeVisible()
await expect(
page.getByText(
'Raclette shoreditch before they sold out lyft. Ethical bicycle rights meh prism '
)
).toBeVisible()
await expect(
page.getByText(
"I'm baby single- origin coffee kickstarter lo - fi paleo skateboard.Tumblr hasht"
)
).toBeVisible()

// CSS checks. We saw this break when we switched bundlers, so while it's not comprehensive, it's at least something.
// While playwright recommends against using locators that are too-closely tied to the DOM, `#redwood-app` is a stable ID.
const bgBlue700 = 'rgb(29, 78, 216)'
expect(
await page
.locator('#redwood-app > header')
.evaluate((e) => window.getComputedStyle(e).backgroundColor)
).toBe(bgBlue700)
expect(page.locator('#redwood-app > header')).toHaveCSS(
'background-color',
bgBlue700
)

const textBlue400 = 'rgb(96, 165, 250)'
expect(
await page
.locator('header a')
.filter({ hasText: 'Redwood Blog' })
.evaluate((e) => window.getComputedStyle(e).color)
).toBe(textBlue400)

// Click text=About
await page.click('text=About')
expect(await page.getByRole('link', { name: 'Redwood Blog' })).toHaveCSS(
'color',
textBlue400
)

// Check the about page.
await page.getByRole('link', { name: 'About', exact: true }).click()
expect(page.url()).toBe('http://localhost:8910/about')

await page.textContent(
'text=This site was created to demonstrate my mastery of Redwood: Look on my works, ye'
await page.getByText(
'This site was created to demonstrate my mastery of Redwood: Look on my works, ye'
)
// Click text=Contact
await page.click('text=Contact')

// Check the contact us page.
await page.getByRole('link', { name: 'Contact Us' }).click()
expect(page.url()).toBe('http://localhost:8910/contact')

// Click text=Admin
await page.click('text=Admin')
// Check the admin page.
await page.getByRole('link', { name: 'Admin' }).click()
expect(page.url()).toBe('http://localhost:8910/posts')
}

Expand Down

0 comments on commit b3ed80d

Please sign in to comment.