From 24db862a19e6772a65a45b62154fd871b8fbe662 Mon Sep 17 00:00:00 2001 From: Malash Date: Sat, 26 Nov 2022 23:47:32 +0800 Subject: [PATCH 1/2] refactor template reuse & fix getSubtreeId --- packages/core/src/components/subtree.ts | 11 +++-- packages/core/src/constants.ts | 2 - packages/core/src/index.ts | 9 +++- .../reconciler/__tests__/instance.test.tsx | 14 +++--- packages/core/src/reconciler/instance.ts | 35 ++++++++------- .../webpack-plugin/src/constants/features.ts | 8 +++- packages/webpack-plugin/src/plugins/bridge.ts | 21 ++++----- .../src/templates/commons/wrapped.ts | 12 +++--- .../__snapshots__/children.wxml.test.tsx.snap | 4 +- .../__tests__/children.wxml.test.tsx | 28 ++---------- .../components/__tests__/wrapped.js.test.tsx | 10 +++-- .../src/templates/components/children.wxml.ts | 32 +++++++------- .../components/components.wxml/index.tsx | 43 +++++++++++-------- .../src/templates/components/item.wxml.ts | 21 +++++---- .../components/leaf-components.wxml.ts | 7 ++- .../src/templates/components/subtree.wxml.ts | 19 +++++--- .../src/templates/components/wrapped.js.ts | 3 +- .../src/templates/components/wrapped.wxml.ts | 14 ++++-- 18 files changed, 145 insertions(+), 148 deletions(-) diff --git a/packages/core/src/components/subtree.ts b/packages/core/src/components/subtree.ts index 5dc7dd70..9629f1c1 100644 --- a/packages/core/src/components/subtree.ts +++ b/packages/core/src/components/subtree.ts @@ -1,18 +1,17 @@ import React, { createElement, Fragment, CSSProperties } from 'react'; -import { TYPE_SUBTREE, GOJI_TARGET } from '../constants'; +import { TYPE_SUBTREE } from '../constants'; -export const useSubtree = GOJI_TARGET === 'wechat' || GOJI_TARGET === 'qq'; +export const useSubtree = process.env.GOJI_TARGET === 'wechat' || process.env.GOJI_TARGET === 'qq'; -export const subtreeMaxDepth = (process.env.GOJI_MAX_DEPTH as any as number) ?? 10; +export const getSubtreeMaxDepthFromConfig = (process.env.GOJI_MAX_DEPTH as any as number) ?? 10; export const Subtree = ({ unsafe_className: className, unsafe_style: style, ...restProps -}: // eslint-disable-next-line camelcase -React.PropsWithChildren<{ unsafe_className?: string; unsafe_style?: CSSProperties }>) => { +}: React.PropsWithChildren<{ unsafe_className?: string; unsafe_style?: CSSProperties }>) => { if (useSubtree) { - return createElement(useSubtree ? TYPE_SUBTREE : Fragment, { + return createElement(TYPE_SUBTREE, { className, style, ...restProps, diff --git a/packages/core/src/constants.ts b/packages/core/src/constants.ts index e628ee8e..e9aa3ccd 100644 --- a/packages/core/src/constants.ts +++ b/packages/core/src/constants.ts @@ -4,8 +4,6 @@ export const TYPE_SUBTREE = 'GOJI_TYPE_SUBTREE'; export type GojiTarget = 'wechat' | 'baidu' | 'alipay' | 'toutiao' | 'qq' | 'toutiao'; -export const GOJI_TARGET: GojiTarget = (process.env.GOJI_TARGET as GojiTarget) || 'wechat'; - export interface SimplifyComponent { name: string; properties: Array; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index c188afaf..21069ccd 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -1,7 +1,7 @@ import { ReactNode } from 'react'; import './patchGlobalObject'; import { createAdaptor, AdaptorType, ExportComponentMeta } from './adaptor'; -import { GOJI_TARGET } from './constants'; +import { GojiTarget } from './constants'; import { batchedUpdates } from './reconciler'; interface RenderOptions { @@ -21,7 +21,12 @@ export const render = (element: ReactNode, options: Partial = {}) ...DEFAULT_RENDER_OPTIONS, ...options, }; - const adaptor = createAdaptor(type, GOJI_TARGET, exportMeta, disablePageSharing); + const adaptor = createAdaptor( + type, + process.env.GOJI_TARGET as GojiTarget, + exportMeta, + disablePageSharing, + ); return adaptor.run(element); }; diff --git a/packages/core/src/reconciler/__tests__/instance.test.tsx b/packages/core/src/reconciler/__tests__/instance.test.tsx index 0a4cb95e..3dd8e31e 100644 --- a/packages/core/src/reconciler/__tests__/instance.test.tsx +++ b/packages/core/src/reconciler/__tests__/instance.test.tsx @@ -1,7 +1,7 @@ import React, { useState, createRef } from 'react'; import { ElementInstance, ElementNodeDevelopment, TextNodeDevelopment } from '../instance'; import { Container } from '../../container'; -import { TYPE_SUBTREE } from '../../constants'; +import { GOJI_VIRTUAL_ROOT, TYPE_SUBTREE } from '../../constants'; import { View, gojiEvents } from '../..'; import { act } from '../../testUtils'; import { batchedUpdates } from '..'; @@ -9,11 +9,6 @@ import { PublicInstance } from '../publicInstance'; import { render } from '../../__tests__/helpers'; import { TestingAdaptorInstance } from '../../__tests__/helpers/adaptor'; -jest.mock('../../../src/components/subtree', () => ({ - useSubtree: true, - subtreeMaxDepth: 5, -})); - describe('ElementInstance', () => { const instance = new ElementInstance( 'view', @@ -66,7 +61,8 @@ describe('ElementInstance', () => { */ const linkElements = (elements: Array) => { const mockContainer = new Container(new TestingAdaptorInstance()); - elements[0].setParent(mockContainer); + const mockRoot = new ElementInstance(GOJI_VIRTUAL_ROOT, {}, [], mockContainer); + elements[0].setParent(mockRoot); for (let i = 0; i < elements.length - 1; i += 1) { elements[i].appendChild(elements[i + 1]); } @@ -75,6 +71,7 @@ describe('ElementInstance', () => { describe('wechat', () => { beforeAll(() => { process.env.GOJI_TARGET = 'wechat'; + process.env.GOJI_MAX_DEPTH = 5 as any; }); test('container works', () => { @@ -123,9 +120,10 @@ describe('ElementInstance', () => { describe('non-wechat', () => { beforeAll(() => { process.env.GOJI_TARGET = 'baidu'; + process.env.GOJI_MAX_DEPTH = 5 as any; }); - test('subtree id should be always undefined', () => { + test.only('subtree id should be always undefined', () => { let leaf: ElementInstance; const elements = [ view(), diff --git a/packages/core/src/reconciler/instance.ts b/packages/core/src/reconciler/instance.ts index 09328d56..c855d76f 100644 --- a/packages/core/src/reconciler/instance.ts +++ b/packages/core/src/reconciler/instance.ts @@ -1,11 +1,11 @@ import { Container } from '../container'; -import { TYPE_TEXT, TYPE_SUBTREE, getTemplateIds } from '../constants'; +import { TYPE_TEXT, TYPE_SUBTREE, getTemplateIds, GOJI_VIRTUAL_ROOT } from '../constants'; import { gojiEvents } from '../events'; import { getNextInstanceId } from '../utils/id'; import { styleAttrStringify } from '../utils/styleAttrStringify'; import { findSimplifyId } from '../utils/simplify'; import { shallowEqual } from '../utils/shallowEqual'; -import { subtreeMaxDepth } from '../components/subtree'; +import { getSubtreeMaxDepthFromConfig, useSubtree } from '../components/subtree'; import { batchedUpdates } from '.'; // prop types from ComponentDesc @@ -59,9 +59,9 @@ export abstract class BaseInstance { public id: number; - protected parent?: Container | ElementInstance; + protected parent?: ElementInstance; - public setParent(parent: Container | ElementInstance | undefined) { + public setParent(parent: ElementInstance | undefined) { this.parent = parent; } @@ -83,9 +83,6 @@ const removeChildrenFromProps = (props: InstanceProps) => { return props; }; -const shouldUseSubtree = () => - process.env.GOJI_TARGET === 'wechat' || process.env.GOJI_TARGET === 'qq'; - export class ElementInstance extends BaseInstance { public constructor( public override type: string, @@ -106,11 +103,8 @@ export class ElementInstance extends BaseInstance { public subtreeDepth?: number; public getSubtreeId(): number | undefined { - // FIXME: For now, only WeChat and QQ have subtree while other platforms render all elements in one page - // We should consider enabling non-WeChat-like subtree/wrapped components support - if (!shouldUseSubtree()) { - return undefined; - } + // eslint-disable-next-line react-hooks/rules-of-hooks + const subtreeMaxDepth = useSubtree ? getSubtreeMaxDepthFromConfig : Infinity; // wrapped component should return its wrapper as subtree id // `process.env.GOJI_WRAPPED_COMPONENTS` is generated from `@goji/webpack-plugin` to tell which // components are wrapped as custom components @@ -119,11 +113,11 @@ export class ElementInstance extends BaseInstance { return this.id; } const ancestors: Array = []; - let cursor: ElementInstance | Container | undefined = this.parent; + let cursor: ElementInstance | undefined = this.parent; // topGojiId === undefined : no from this element to container // topGojiId === cursor.id : the id of closest let topGojiId: number | undefined; - while (!(cursor instanceof Container)) { + while (cursor?.type !== GOJI_VIRTUAL_ROOT) { if (!cursor) { console.warn( 'Cannot find parent in ElementInstance. This might be an internal error in GojiJS.', @@ -131,20 +125,25 @@ export class ElementInstance extends BaseInstance { return undefined; } // wrapped component creates a new subtree - if (cursor.type === TYPE_SUBTREE || wrappedComponentsFromWebpack.includes(cursor.type)) { + if ( + // eslint-disable-next-line react-hooks/rules-of-hooks + (useSubtree && cursor.type === TYPE_SUBTREE) || + wrappedComponentsFromWebpack.includes(cursor.type) + ) { topGojiId = cursor.id; break; } ancestors.unshift(cursor); cursor = cursor.parent; } + // find the closest subtree from this element to root const subtreePosition = ancestors.length - (ancestors.length % subtreeMaxDepth) - 1; const subtreeElement = ancestors[subtreePosition]; - if (!subtreeElement) { - return topGojiId; + if (subtreeElement) { + return subtreeElement.id; } - return subtreeElement.id; + return topGojiId; } public pure(path: string, parentTag?: UpdateType): [ElementNode, Updates] { diff --git a/packages/webpack-plugin/src/constants/features.ts b/packages/webpack-plugin/src/constants/features.ts index 6fb7c88f..88942214 100644 --- a/packages/webpack-plugin/src/constants/features.ts +++ b/packages/webpack-plugin/src/constants/features.ts @@ -3,10 +3,14 @@ import { GojiTarget } from '@goji/core'; export const getFeatures = (target: GojiTarget) => ({ useSubtree: target === 'wechat' || target === 'qq', - // alipay only support recursion dependency self to self so we have to inline the children.wxml + // Alipay only support recursion dependency self to self so we have to inline the children.wxml // Success: A -> A -> A // Fail: A -> B -> A - useInlineChildren: target === 'alipay', + useInlineChildrenInComponent: target === 'alipay', + + // Baidu fails to render if an outside same `` exists + // https://github.com/airbnb/goji-js/issues/185 + useInlineChildrenInItem: target === 'baidu', // Baidu doesn't support `template` inside `text` so we need to flat text manually useFlattenText: target === 'baidu', diff --git a/packages/webpack-plugin/src/plugins/bridge.ts b/packages/webpack-plugin/src/plugins/bridge.ts index 52745f30..37a6e186 100644 --- a/packages/webpack-plugin/src/plugins/bridge.ts +++ b/packages/webpack-plugin/src/plugins/bridge.ts @@ -13,7 +13,7 @@ import { GojiBasedWebpackPlugin } from './based'; import { minimize } from '../utils/minimize'; import { getSubpackagesInfo, findBelongingSubPackage, MAIN_PACKAGE } from '../utils/config'; import { renderTemplate } from '../templates'; -import { componentWxml } from '../templates/components/components.wxml'; +import { componentWxml, useSubtreeAsChildren } from '../templates/components/components.wxml'; import { childrenWxml } from '../templates/components/children.wxml'; import { leafComponentWxml } from '../templates/components/leaf-components.wxml'; import { itemWxml } from '../templates/components/item.wxml'; @@ -80,8 +80,8 @@ export class GojiBridgeWebpackPlugin extends GojiBasedWebpackPlugin { path.posix.join(basedir, BRIDGE_OUTPUT_DIR, `children${depth}.wxml`), () => childrenWxml({ - maxDepth, - componentsDepth: depth + 1, + relativePathToBridge: '.', + componentDepth: depth, }), ); await this.renderTemplateComponentToAsset( @@ -89,8 +89,8 @@ export class GojiBridgeWebpackPlugin extends GojiBasedWebpackPlugin { path.posix.join(basedir, BRIDGE_OUTPUT_DIR, `components${depth}.wxml`), () => componentWxml({ - depth, - componentsDepth: depth + 1, + componentDepth: depth, + childrenDepth: depth + 1 === maxDepth ? useSubtreeAsChildren : depth + 1, components, simplifiedComponents, pluginComponents, @@ -148,8 +148,8 @@ export class GojiBridgeWebpackPlugin extends GojiBasedWebpackPlugin { path.posix.join(basedir, BRIDGE_OUTPUT_DIR, `components0.wxml`), () => componentWxml({ - depth: 0, - componentsDepth: 0, + componentDepth: 0, + childrenDepth: 0, components, simplifiedComponents, pluginComponents, @@ -164,8 +164,8 @@ export class GojiBridgeWebpackPlugin extends GojiBasedWebpackPlugin { path.posix.join(basedir, BRIDGE_OUTPUT_DIR, `children0.wxml`), () => childrenWxml({ - maxDepth: Infinity, - componentsDepth: 0, + relativePathToBridge: '.', + componentDepth: 0, }), ); } @@ -223,9 +223,6 @@ export class GojiBridgeWebpackPlugin extends GojiBasedWebpackPlugin { await this.renderSubtreeBridge(compilation, bridgeBasedirs); // render subtree component await this.renderChildrenRenderComponent(compilation, bridgeBasedirs); - } else if (getFeatures(this.options.target).useInlineChildren) { - // render component0 with inlined children0 - await this.renderComponentTemplate(compilation, bridgeBasedirs); } else { // render component0 await this.renderComponentTemplate(compilation, bridgeBasedirs); diff --git a/packages/webpack-plugin/src/templates/commons/wrapped.ts b/packages/webpack-plugin/src/templates/commons/wrapped.ts index 835bb4e5..b1baedf5 100644 --- a/packages/webpack-plugin/src/templates/commons/wrapped.ts +++ b/packages/webpack-plugin/src/templates/commons/wrapped.ts @@ -59,17 +59,15 @@ export const WRAPPED_CONFIGS: Record = { const ids = getIds(); return t` - - -