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

feat: expose Fragment on h #2846

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

simonhaenisch
Copy link
Contributor

This way it's possible to set jsxFragmentFactory to h.Fragment in tsconfig, so that <></> syntax can be used without having to import Fragment explicitly (just importing h is sufficient).

This way it's possible to set `jsxFragmentFactory` to `h.Fragment` in tsconfig, so that `<></>` syntax can be used without having to import `Fragment` explicitly (just importing `h` is sufficient).
@simonhaenisch simonhaenisch marked this pull request as draft March 9, 2021 10:04
@simonhaenisch simonhaenisch marked this pull request as ready for review March 9, 2021 10:19
@simonhaenisch
Copy link
Contributor Author

The broken test is from eec7651 (see https://github.com/ionic-team/stencil/runs/1796291494). All other tests pass locally.

@manucorporat
Copy link
Contributor

This is kind of cool!, i like it. My only concern is so performance regression because adding a property to an arrow function is disables some V8 optimization.

Wondering how we could check that

@simonhaenisch
Copy link
Contributor Author

Interesting, I didn't know that. Is this different when using a normal function h instead of assigning an anonymous arrow function to h? Do you have a link that explains this optimization? Is there a way to measure the impact of this?

BTW In the meantime what I did was to patch h at runtime with a globalScript.

import { h, Fragment } from '@stencil/core;

export default () => {
  if (!('Fragment' in h)) {
    h.Fragment = Fragment;
  }
}

(breaks spec tests with newSpecPage though)

@simonhaenisch
Copy link
Contributor Author

simonhaenisch commented Mar 18, 2021

Hm, idea:

const h = () => { /* ... */ }

export default h

export const Fragment = (_, children) => children
import h from './h'

export { h }

@simonhaenisch
Copy link
Contributor Author

Never mind, mixing default and named exports doesn't seem to work with Rollup (see repl).

@simonhaenisch simonhaenisch requested a review from a team October 6, 2021 16:35
@simonhaenisch simonhaenisch requested a review from a team as a code owner May 13, 2024 16:20
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.

None yet

2 participants