Conversation
| export type { AnimationSettings } from '../src/core/animations/CoreAnimation.js'; | ||
| export type { TimingFunction } from '../src/core/utils.js'; | ||
| export type { Inspector } from '../src/main-api/Inspector.js'; | ||
| export type { CoreNodeRenderState } from '../src/core/CoreNode.js'; |
There was a problem hiding this comment.
I'd also like to report something here.
Is it intended for consumers of lightningjs/renderer to be able to do:
import { CoreNodeRenderState } from '@lightningjs/renderer';
if (renderState === CoreNodeRenderState.InViewport) { ... }?
Because if so, then this isn't working. All enums are being exported as export declare enum X. In order to preserve enums for consumers, preserveConstEnums must be used.
|
@guilhermesimoes I think you aren't approaching that correctly - to create nodes you should be go through the
Both the official Blits and unofficial SolidJS frameworks use this API, and in both cases they don't directly extend those objects, which is potentially a departure from the Lightning 2 approach where you can extend the base Element type. |
|
The entire Peacock codebase is written in a class based approach, more or less like this: import { Component, Element } from '@lightningjs/core';
export class SomePage extends Component {
private title: Element;
constructor(stage: Stage) {
super(stage);
const title = (this.title = new Element(stage));
title.y = SOME_PAGE_TITLE_Y;
title.x = HORIZONTAL_SAFE_ZONE;
title.text = 'Hello World';
}
}As you can imagine, migrating over 300 components is not an easy task. We could create our own import {
type Stage,
type CoreNode // note, this type import isn't even available
} from '@lightningjs/renderer';
export class Element {
private node: CoreNode;
constructor(stage: Stage, props = {} as CoreNodeProps) {
this.node = stage.createNode(props);
}
set x(x) {
this.node.x = x;
}
get x() {
return this.node.x;
}
// etc ...
}But this approach has several disadvantages:
As such, we believe a better approach for a progressive migration from Lightning 2 to 3 is like so: import {
type Stage,
type CoreNodeProps, // we also need this type import, which is also not available
CoreNode
} from '@lightningjs/renderer';
export class Element extends CoreNode {
constructor(stage: Stage, props = {} as CoreNodeProps) {
super(stage, stage.resolveNodeDefaults(props));
}
}This approach:
We understand this may not fit in the Lightning team's vision of a declarative approach such as SolidJS or Blits, which at build time transform templates into nested calls to |
|
@guilhermesimoes I see, so you're using the classic LightningJS 2 (L2) Component class but without templating. Let me clarify a bit: the LightningJS 3 (L3) renderer CoreNode/CoreTextNode do NOT correspond to L2 Element class - they correspond to L2 ElementCore, which the Element class internally creates and to which it forwards most properties. As of now, there is no shortcut to migrating to L3. The LightningJS team has a limited capacity and they focus on the new Blits UI framework. While I think it should be possible to reproduce some or all of L2 Application/Component/Element API using the core L3 renderer, that's a difficult project with little benefit ultimately: if your app works well with L2 engine then that's OK - it wouldn't run better by swapping the internals with L3 renderer. |
Yes, we know. We are aware.
I'm confused. You don't want teams to upgrade to L3?
We understand that. We're not looking for shortcuts. For starters, we only need these very minor changes (2 class exports + 3 type exports). Is adding these exports a high maintenance burden to the LightningJS team? |
|
Disclaimer: I'm not part of the team, so I can't comment on the decisions and priorities. I don't think the export change and trying to extend the code nodes is wise though. FWIW if you want to eventually migrate to Blits or SolidJS, you need to follow the approach where the code nodes (created using the |
I already explained in #760 (comment) why that is a worse approach.
Oh, I had not realised that. I will wait for the team's input then. |
|
I noted that you find it worse (and I don't necessarily disagree), but the reality is that it's how L2 Component/Element and L3's Blits and SolidJS frameworks work, so if you think in the future you'll use these frameworks, you won't escape the extra object pattern. I think it would potentially help supporting your request if you could share a POC app using the modified API demonstrating how you can tie everything together. |
|
Hey @guilhermesimoes 👋 Super excited to see Peacock dive into L3! We're on a similar track spinning up ION with @estobbart for FireTV/Consoles collaboration effort across the Sky/Comcast and the NBCU family. If you need us, we're basically standing by with snacks and support. Your proposed approach looks good to me, totally reasonable for a transition period. Would love to see a small adaptation/transition layer in action. You can always re-evaluate how to go beyond this, making large scale changes to an existing app is always a big challenge. @jfboeve … can we move this forward?
True, priority usually goes to internal work where goals align. Open source still matters, we just occasionally have to juggle team/department/company-wide and open source priorities.
To be clear: yes, we absolutely want teams to upgrade to L3. We've seen major, measurable improvements across Xfinity, Rogers, and Sky apps — across a wide range of devices. Our group is heavily committed to consolidating and moving more L2 apps onto L3. Some teams (cough HBO cough, love you guys 😇 ) are taking their time on L2 and that's their prerogative. But don't let that slow you down. You're part of the family, and we're here to support your move toward L3. |
That's great to hear!
Awesome news, can't wait to take L3 to production and give our costumers an even faster app.
We patched We will certainly re-evaluate our approach as we progress. |
Add type exports
TextRenderer,CoreNodePropsandCoreTextNodeProps.Also export
CoreNodeandCoreTextNodesince we can't usestage.createNode()orstage.createTextNode().Peacock has been using Lightning v2's
Elementfor several years now, so all our components extend that class. In order to migrate to Lightning v3, we need access to the new building blocks,CoreNodeandCoreTextNode, so that our components can extend those.