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

fix(engine-dom): support LWC v5 compiler slot format #3973

Merged
merged 6 commits into from
Feb 6, 2024
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: 2 additions & 0 deletions .github/workflows/karma.yml
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,8 @@ jobs:
- run: DISABLE_SYNTHETIC_SHADOW_SUPPORT_IN_COMPILER=1 DISABLE_SYNTHETIC=1 yarn sauce:ci
- run: DISABLE_STATIC_CONTENT_OPTIMIZATION=1 yarn sauce:ci
- run: DISABLE_STATIC_CONTENT_OPTIMIZATION=1 DISABLE_SYNTHETIC=1 yarn sauce:ci
- run: FORCE_LWC_V5_COMPILER_FOR_TEST=1 yarn sauce:ci
- run: FORCE_LWC_V5_COMPILER_FOR_TEST=1 DISABLE_SYNTHETIC=1 yarn sauce:ci
- run: NODE_ENV_FOR_TEST=production yarn sauce:ci
- run: NODE_ENV_FOR_TEST=production DISABLE_SYNTHETIC=1 yarn sauce:ci
- run: yarn hydration:sauce:ci
Expand Down
12 changes: 11 additions & 1 deletion packages/@lwc/engine-core/src/framework/api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ import { logError } from '../shared/logger';

import { invokeEventListener } from './invoker';
import { getVMBeingRendered, setVMBeingRendered } from './template';
import { EmptyArray } from './utils';
import { applyTemporaryCompilerV5SlotFix, EmptyArray } from './utils';
import { isComponentConstructor } from './def';
import { RenderMode, ShadowMode, SlotSet, VM } from './vm';
import { LightningElementConstructor } from './base-lightning-element';
Expand Down Expand Up @@ -163,6 +163,9 @@ function h(sel: string, data: VElementData, children: VNodes = EmptyArray): VEle
});
}

// TODO [#3974]: remove temporary logic to support v5 compiler + v6+ engine
data = applyTemporaryCompilerV5SlotFix(data);
nolanlawson marked this conversation as resolved.
Show resolved Hide resolved

const { key, slotAssignment } = data;

const vnode: VElement = {
Expand Down Expand Up @@ -215,6 +218,9 @@ function s(
const vmBeingRendered = getVMBeingRendered()!;
const { renderMode, apiVersion } = vmBeingRendered;

// TODO [#3974]: remove temporary logic to support v5 compiler + v6+ engine
data = applyTemporaryCompilerV5SlotFix(data);

if (
!isUndefined(slotset) &&
!isUndefined(slotset.slotAssignments) &&
Expand Down Expand Up @@ -346,6 +352,10 @@ function c(
});
}
}

// TODO [#3974]: remove temporary logic to support v5 compiler + v6+ engine
data = applyTemporaryCompilerV5SlotFix(data);

const { key, slotAssignment } = data;
let elm, aChildren, vm;
const vnode: VCustomElement = {
Expand Down
27 changes: 27 additions & 0 deletions packages/@lwc/engine-core/src/framework/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,13 @@ import {
seal,
isAPIFeatureEnabled,
APIFeature,
isUndefined,
isNull,
} from '@lwc/shared';
import { StylesheetFactory, TemplateStylesheetFactories } from './stylesheet';
import { getComponentAPIVersion } from './component';
import { LightningElementConstructor } from './base-lightning-element';
import { VElementData } from './vnodes';

type Callback = () => void;

Expand Down Expand Up @@ -129,3 +132,27 @@ export function assertNotProd() {
throw new ReferenceError();
}
}

// Temporary fix for when the LWC v5 compiler is used in conjunction with a v6+ engine
// The old compiler format used the "slot" attribute in the `data` bag, whereas the new
// format uses the special `slotAssignment` key.
// This should be removed when the LWC v5 compiler is not used anywhere where it could be mismatched
// with another LWC engine version.
// TODO [#3974]: remove temporary logic to support v5 compiler + v6+ engine
export function applyTemporaryCompilerV5SlotFix(data: VElementData) {
if (lwcRuntimeFlags.DISABLE_TEMPORARY_V5_COMPILER_SUPPORT) {
return data;
}
const { attrs } = data;
if (!isUndefined(attrs)) {
const { slot } = attrs;
if (!isUndefined(slot) && !isNull(slot)) {
return {
...data,
attrs: cloneAndOmitKey(attrs, 'slot'),
slotAssignment: String(slot),
};
}
}
return data;
}
1 change: 1 addition & 0 deletions packages/@lwc/features/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ const features: FeatureFlagMap = {
ENABLE_FROZEN_TEMPLATE: null,
ENABLE_LEGACY_SCOPE_TOKENS: null,
ENABLE_FORCE_SHADOW_MIGRATE_MODE: null,
DISABLE_TEMPORARY_V5_COMPILER_SUPPORT: null,
};

if (!(globalThis as any).lwcRuntimeFlags) {
Expand Down
5 changes: 5 additions & 0 deletions packages/@lwc/features/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,11 @@ export interface FeatureFlagMap {
* If true, enable experimental shadow DOM migration mode globally.
*/
ENABLE_FORCE_SHADOW_MIGRATE_MODE: FeatureFlagValue;

/**
* If true, disable temporary support for the LWC v5 compiler format.
*/
DISABLE_TEMPORARY_V5_COMPILER_SUPPORT: FeatureFlagValue;
}

export type FeatureFlagName = keyof FeatureFlagMap;
10 changes: 10 additions & 0 deletions packages/@lwc/integration-karma/helpers/test-setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,16 @@ function patchConsole() {
var originalMethod = console[method];
// eslint-disable-next-line no-console
console[method] = function () {
// TODO [#3974]: remove temporary logic to support v5 compiler + v6+ engine
if (
process.env.FORCE_LWC_V5_COMPILER_FOR_TEST &&
arguments[0] &&
arguments[0].includes('template was compiled with v5')
) {
// ignore this warning; this is expected
return;
}

consoleCallCount++;
return originalMethod.apply(this, arguments);
};
Expand Down
1 change: 1 addition & 0 deletions packages/@lwc/integration-karma/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"@lwc/engine-dom": "6.1.0",
"@lwc/engine-server": "6.1.0",
"@lwc/rollup-plugin": "6.1.0",
"@lwc/rollup-plugin-v5": "npm:@lwc/rollup-plugin@5.3.0",
"@lwc/synthetic-shadow": "6.1.0",
"chokidar": "^3.5.3",
"istanbul-lib-coverage": "^3.2.2",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ const {
DISABLE_STATIC_CONTENT_OPTIMIZATION,
NODE_ENV_FOR_TEST,
API_VERSION,
FORCE_LWC_V5_COMPILER_FOR_TEST,
} = require('../../shared/options');

// These are used to decide the directory that coverage is written to
Expand All @@ -30,6 +31,7 @@ const TAGS = [
DISABLE_STATIC_CONTENT_OPTIMIZATION && 'disable-static-content-optimization',
`NODE_ENV-${NODE_ENV_FOR_TEST}`,
API_VERSION && `api-version-${API_VERSION}`,
FORCE_LWC_V5_COMPILER_FOR_TEST && 'force-lwc-v5-compiler',
].filter(Boolean);

module.exports = TAGS;
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ const {
ENABLE_SYNTHETIC_SHADOW_IN_HYDRATION,
NODE_ENV_FOR_TEST,
API_VERSION,
FORCE_LWC_V5_COMPILER_FOR_TEST,
} = require('../shared/options');

const DIST_DIR = path.resolve(__dirname, '../../dist');
Expand All @@ -48,7 +49,8 @@ function createEnvFile() {
ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL: ${ENABLE_ARIA_REFLECTION_GLOBAL_POLYFILL},
ENABLE_SYNTHETIC_SHADOW_IN_HYDRATION: ${ENABLE_SYNTHETIC_SHADOW_IN_HYDRATION},
LWC_VERSION: ${JSON.stringify(LWC_VERSION)},
API_VERSION: ${JSON.stringify(API_VERSION)}
API_VERSION: ${JSON.stringify(API_VERSION)},
FORCE_LWC_V5_COMPILER_FOR_TEST: ${JSON.stringify(FORCE_LWC_V5_COMPILER_FOR_TEST)}
}
};
`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,8 @@
'use strict';

const path = require('path');

const { rollup } = require('rollup');
const lwcRollupPlugin = require('@lwc/rollup-plugin');
Copy link
Contributor

Choose a reason for hiding this comment

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

sanity check: are none of the slot-related tests in @lwc/integration-tests affected by this slot format change? That project's build.js has its own separate entry point.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's possible that @lwc/integration-tests is affected, but I'm hoping the Karma tests are sufficient. The @lwc/integration-tests are not terribly well-maintained at this point.

const { FORCE_LWC_V5_COMPILER_FOR_TEST } = require('../shared/options.js');

const {
DISABLE_SYNTHETIC_SHADOW_SUPPORT_IN_COMPILER,
Expand Down Expand Up @@ -50,6 +49,11 @@ function createPreprocessor(config, emitter, logger) {
// TODO [#3370]: remove experimental template expression flag
const experimentalComplexExpressions = suiteDir.includes('template-expressions');

// TODO [#3974]: remove temporary logic to support v5 compiler + v6+ engine
const lwcRollupPlugin = FORCE_LWC_V5_COMPILER_FOR_TEST
? require('@lwc/rollup-plugin-v5')
: require('@lwc/rollup-plugin');

const plugins = [
lwcRollupPlugin({
// Sourcemaps don't work with Istanbul coverage
Expand Down
4 changes: 4 additions & 0 deletions packages/@lwc/integration-karma/scripts/shared/options.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ const API_VERSION = process.env.API_VERSION
? parseInt(process.env.API_VERSION, 10)
: HIGHEST_API_VERSION;

// TODO [#3974]: remove temporary logic to support v5 compiler + v6+ engine
const FORCE_LWC_V5_COMPILER_FOR_TEST = Boolean(process.env.FORCE_LWC_V5_COMPILER_FOR_TEST);

module.exports = {
// Test configuration
LEGACY_BROWSERS,
Expand All @@ -44,6 +47,7 @@ module.exports = {
DISABLE_STATIC_CONTENT_OPTIMIZATION,
SYNTHETIC_SHADOW_ENABLED: !DISABLE_SYNTHETIC,
API_VERSION,
FORCE_LWC_V5_COMPILER_FOR_TEST,
ENABLE_SYNTHETIC_SHADOW_IN_HYDRATION,
GREP: process.env.GREP,
COVERAGE: Boolean(process.env.COVERAGE),
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { createElement } from 'lwc';
import { createElement, setFeatureFlagForTest } from 'lwc';
import { extractDataIds } from 'test-utils';

import { vFragBookEndEnabled, lightDomSlotForwardingEnabled } from 'test-utils';
Expand All @@ -21,16 +21,6 @@ function createTestElement(tag, component) {
}

describe('Slotting', () => {
it('should render properly', () => {
const nodes = createTestElement('x-default-slot', BasicSlot);

expect(Array.from(nodes['x-container'].children)).toEqual([
nodes['upper-text'],
nodes['default-text'],
nodes['lower-text'],
]);
});

it('should render dynamic children', async () => {
const nodes = createTestElement('x-dynamic-children', DynamicChildren);
expect(Array.from(nodes['x-light-container'].children)).toEqual([
Expand Down Expand Up @@ -137,4 +127,29 @@ describe('Slotting', () => {
expect(commentNodes.length).toBe(6); // 3 slots, so 3*2=6 comment nodes
}
});

// TODO [#3974]: remove temporary logic to support v5 compiler + v6+ engine
if (process.env.FORCE_LWC_V5_COMPILER_FOR_TEST) {
describe('DISABLE_TEMPORARY_V5_COMPILER_SUPPORT', () => {
beforeEach(() => {
setFeatureFlagForTest('DISABLE_TEMPORARY_V5_COMPILER_SUPPORT', true);
});

afterEach(() => {
setFeatureFlagForTest('DISABLE_TEMPORARY_V5_COMPILER_SUPPORT', false);
});

it('should not render slots properly', () => {
const nodes = createTestElement('x-default-slot', BasicSlot);

expect(Array.from(nodes['x-container'].children)).toEqual([
nodes['container-upper-slot-default'],
nodes['upper-text'],
nodes['default-text'],
nodes['lower-text'],
nodes['container-lower-slot-default'],
]);
});
});
}
});
Loading
Loading