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

chore: remove commonjs bundle split points #417

Merged
merged 4 commits into from
Dec 20, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@
},
{
"path": "./packages/react/dist/browser/runtime.js",
"limit": "489B",
"limit": "445B",
Copy link
Contributor

Choose a reason for hiding this comment

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

Noice ❤️

"import": "{ CC, CS }",
"ignore": [
"react"
Expand Down
46 changes: 20 additions & 26 deletions packages/react/src/__tests__/server-side-hydrate.test.tsx
Original file line number Diff line number Diff line change
@@ -1,31 +1,21 @@
import React from 'react';
import { renderToString } from 'react-dom/server';
import ReactDOM from 'react-dom';
import { styled } from '@compiled/react';
import { isNodeEnvironment } from '@compiled/react/dist/runtime/is-node';
import { useCache } from '@compiled/react/dist/runtime/provider';
import { hydrate } from 'react-dom';
import { CC, CS } from '../runtime';

jest.mock('@compiled/react/dist/runtime/is-node');
jest.mock('../runtime/is-node', () => ({
isNodeEnvironment: () => false,
}));

describe('server side hydrate', () => {
beforeEach(() => {
jest.spyOn(global.console, 'error');
flushEnvironment('node');
});

const flushEnvironment = (env: 'node' | 'browser') => {
// This isn't a real hook.
// eslint-disable-next-line react-hooks/rules-of-hooks
const cache = useCache();
for (const key in cache) {
// Flush the cache out - unfortunately it persisted between tests.
delete cache[key];
}
(isNodeEnvironment as jest.Mock).mockReturnValue(env === 'node');
const flushRuntimeModule = () => {
jest.resetModules();
// We need to force this module to re-instantiate because on the client
// when it does it will move all found SSRd style elements to the head.
require('@compiled/react/runtime');
require('../runtime');
};

const appendHTML = (markup: string) => {
Expand All @@ -36,16 +26,20 @@ describe('server side hydrate', () => {
};

it('should not log any warnings when hydrating HTML', () => {
const StyledDiv = styled.div`
font-size: 12px;
color: blue;
border: 1px solid black;
`;
// It's notoriously hard to do both server and client rendering in this test.
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

// Instead of doing the server flow we hardcode the result instead.
const elem = appendHTML(
`<style data-cmpld="true">.foo{color:blue}</style><div class="foo">hello world</div>`
);

const element = <StyledDiv>hello world</StyledDiv>;
const app = appendHTML(renderToString(element));
flushEnvironment('browser');
ReactDOM.hydrate(element, app);
flushRuntimeModule();
hydrate(
<CC>
<CS>{['.foo { color: blue; }', '.foo { color: blue; }']}</CS>
<div className="foo">hello world</div>
</CC>,
elem
);

expect(console.error).not.toHaveBeenCalled();
});
Expand Down

This file was deleted.

85 changes: 0 additions & 85 deletions packages/react/src/runtime/__tests__/provider.test.tsx

This file was deleted.

2 changes: 1 addition & 1 deletion packages/react/src/runtime/index.tsx
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
export { default as CS } from './style';
export { default as CC } from './provider';
export { default as CC } from './style-cache';
export { default as ax } from './ax';
12 changes: 0 additions & 12 deletions packages/react/src/runtime/provider/index.tsx

This file was deleted.

51 changes: 0 additions & 51 deletions packages/react/src/runtime/provider/provider-browser.tsx

This file was deleted.

35 changes: 0 additions & 35 deletions packages/react/src/runtime/provider/provider-server.tsx

This file was deleted.

3 changes: 0 additions & 3 deletions packages/react/src/runtime/provider/types.tsx

This file was deleted.

57 changes: 57 additions & 0 deletions packages/react/src/runtime/style-cache.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import * as React from 'react';
import { createContext, useContext } from 'react';
import { isNodeEnvironment } from './is-node';
import { ProviderComponent, UseCacheHook } from './types';

/**
* Cache to hold already used styles.
* React Context on the server - singleton object on the client.
*/
const Cache: any = isNodeEnvironment() ? createContext<Record<string, true> | null>(null) : {};

if (!isNodeEnvironment()) {
/**
* Iterates through all found style elements generated when server side rendering.
*
* @param cb
*/
const ssrStyles = document.querySelectorAll<HTMLStyleElement>('style[data-cmpld]');
for (let i = 0; i < ssrStyles.length; i++) {
// Move all found server-side rendered style elements to the head before React hydration happens.
document.head.appendChild(ssrStyles[i]);
}
}

/**
* Hook using the cache created on the server or client.
*/
export const useCache: UseCacheHook = () => {
if (isNodeEnvironment()) {
// On the server we use React Context to we don't leak the cache between SSR calls.
// During runtime this hook isn't conditionally called - it is at build time that the flow gets decided.
// eslint-disable-next-line react-hooks/rules-of-hooks
return useContext(Cache) || {};
}

// On the client we use the object singleton.
return Cache;
};

/**
* On the server this ensures the minimal amount of styles will be rendered
* safely using React Context.
*
* On the browser this turns into a fragment with no React Context.
*/
const StyleCacheProvider: ProviderComponent = (props) => {
if (isNodeEnvironment()) {
// This code path isn't conditionally called at build time - safe to ignore.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: 🤔 It will work we know. Just wondering won't it confuse developers who are so familiar with rules of hooks? Is there a way we can split it into two components like we did before and do something like:

const StyleCacheProvider = isNodeEnvironment() ? StyleServerCacheProvider: StyleClientCacheProvider;

or maybe

const StyleCacheProvider: ProviderComponent = (props) => {
  if (isNodeEnvironment()) {
    return <StyleServerCacheProvider {...props} />
  }
  
  return <StyleClientCacheProvider {...props} />
}

Its still difficult to do as above for useCache though 😞

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this keeps the bundlesize down and keeps the react tree as small as possible - i wouldn't want to make more TBH

pros & cons - in this instance having good amounts of comments should work - i'll update them a bit though

Copy link
Contributor

Choose a reason for hiding this comment

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

No worries..hehe 😃

// eslint-disable-next-line react-hooks/rules-of-hooks
const inserted = useCache();
return <Cache.Provider value={inserted}>{props.children}</Cache.Provider>;
}

return props.children as JSX.Element;
};

export default StyleCacheProvider;
2 changes: 1 addition & 1 deletion packages/react/src/runtime/style.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import React, { memo } from 'react';
import insertRule, { getStyleBucketName, styleBucketOrdering } from './sheet';
import { analyzeCssInDev } from './dev-warnings';
import { StyleSheetOpts, Bucket } from './types';
import { useCache } from './provider';
import { useCache } from './style-cache';
import { isNodeEnvironment } from './is-node';

interface StyleProps extends StyleSheetOpts {
Expand Down
4 changes: 4 additions & 0 deletions packages/react/src/runtime/types.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,7 @@ export type Bucket =
| 'a'
// at-rules
| 'm';

export type UseCacheHook = () => Record<string, true>;

export type ProviderComponent = (props: { children: JSX.Element[] | JSX.Element }) => JSX.Element;