Skip to content

feat: Svelte 5#4

Draft
NathanWalker wants to merge 9 commits intomasterfrom
feat/svelte5
Draft

feat: Svelte 5#4
NathanWalker wants to merge 9 commits intomasterfrom
feat/svelte5

Conversation

@NathanWalker
Copy link
Copy Markdown
Member

Comment on lines +116 to +126
addEventListener(target: any, type: string, handler: any, options?: any) {
if (target.addEventListener) {
target.addEventListener(type, handler);
}
},

removeEventListener(target: any, type: string, handler: any, options?: any) {
if (target.removeEventListener) {
target.removeEventListener(type, handler);
}
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is it fine here that the options are discarded?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We don't use options in Nativescript so I think it is fine

Copy link
Copy Markdown
Member Author

@NathanWalker NathanWalker Apr 10, 2026

Choose a reason for hiding this comment

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

Currently it is however @shirakaba has a long standing PR which may standardize that handling eventually: 10100

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't think I'll be returning to that PR, unfortunately. The only path I see to DOM conformance in NativeScript is making a whole new core library – there's too much resistance to breaking changes in @nativescript/core.

As for what we can do here – the options parameter in NativeScript event listeners infamously determines the this context that gets bound to the event handler (and is not treated as AddEventListenerOptions | boolean like it would be in a true DOM EventTarget). There may have been a use-case for that this-binding in the XAML flavour of NativeScript, but every custom renderer since has struggled with it.

It's quite tricky to meaningfully support the options object in userland. We would have to basically reimplement EventTarget at the custom renderer level. It would need to segregate bubbles: true events from bubbles: false events, and keep track of the once?: boolean value. And to really match DOM behaviour, it would have to implement bubbling and capturing phases (NativeScript Core events are more like Node.js EventTarget, in that they only fire at the target and do not propagate).

The easiest would be to avoid fighting NativeScript Core and thus not bother supporting bubbling/capturing in userland. But supporting once?: true is doable and worth doing.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Mmm weird that you need this...wasn't included when installing the package?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It helped resolution on some of the types but will see if can be discarded.

Comment on lines +13 to +24
function fixRendererImport() {
return {
name: 'fix-renderer-import',
generateBundle(options, bundle) {
for (const file of Object.values(bundle)) {
if (file.type === 'chunk' && file.code) {
file.code = file.code.replace(/from ['"]\.\.\/renderer['"]/g, "from './renderer'");
}
}
}
};
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This can technically be solved at the config level by using a function instead of a string or by make the renderer a package itself

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

I think the any's can be replaced...the types are not inferred until the end but once the renderer is complete all the types should be inferred completely

Copy link
Copy Markdown
Member

@farfromrefug farfromrefug left a comment

Choose a reason for hiding this comment

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

It looks great.
We need to look at Template though before merging. It is a must have
Also I need to create a v4 branch first.

@paoloricciuti
Copy link
Copy Markdown

We need to look at Template though before merging. It is a must have

Curious what this is about.

Also, I would wait until we merge the renderer API in svelte first 😅

@NathanWalker
Copy link
Copy Markdown
Member Author

Curious what this is about.

ListViews and other similar native components (Pager's, etc.) utilize recycled rows at the native level and Template handling refers to the items inside those mobile specialized controls.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants