preact-custom-element icon indicating copy to clipboard operation
preact-custom-element copied to clipboard

Add support for shadow DOM closed mode

Open stevenle opened this issue 2 years ago • 2 comments

Fixes #74

stevenle avatar Sep 09 '22 17:09 stevenle

I added two tests (one for {mode: 'open'} and one for {mode: 'closed'}. Would appreciate another look to make sure I'm following the correct patterns for this project.

~~(Note: I needed to update puppeteer in order for it to work on my Macbook with Apple Silicon. npm install wasn't working for me, so I used yarn and so the package-lock.json file wasn't updated.)~~ Edit: Ignore this, I realized npm test:browsers worked without the puppeteer dependency.

stevenle avatar Sep 10 '22 15:09 stevenle

Thanks for the review! What's the next step from here? I assume you'll handle the merge and the publish to npm?

stevenle avatar Sep 11 '22 06:09 stevenle

Hi @marvinhagemeister , what's the next step here. Can we get this out?

AGrabovajFitA avatar Dec 09 '22 10:12 AGrabovajFitA

Hi, any updates on this PR? I would love for this to get merged as we wouldn't have to keep our own fork of this project. 🙏

leifriksheim avatar Apr 06 '23 07:04 leifriksheim

Test CI still fails it seems

MattCCC avatar Jul 05 '23 23:07 MattCCC

@marvinhagemeister if you update this branch it may pass now.

bhollis avatar Aug 15 '23 07:08 bhollis

Nice, the tests are passing!

bhollis avatar Aug 16 '23 05:08 bhollis

Seems like this is not included in the latest version, when can we expect this to get released?

janbiasi avatar Oct 08 '23 20:10 janbiasi

Also need this change and wondering when it will be released!

HowieG avatar Oct 12 '23 09:10 HowieG

@janbiasi dang I just realized the last publish to npm was in 2020 😲 . Haven't encountered this before do we just clone the repo then?

HowieG avatar Oct 12 '23 09:10 HowieG

@janbiasi seems we can change our package.json to grab from the repo directly

{ "dependencies": { "preact-custom-element": "https://github.com/preactjs/preact-custom-element.git" } }

HowieG avatar Oct 12 '23 10:10 HowieG

seems we can change our package.json to grab from the repo directly

@HowieG Well yes, that works as a workaround at least 😄 But I'm really questioning why this feature wasn't released via NPM. Maybe @marvinhagemeister can tell us more details about the official release.

janbiasi avatar Oct 12 '23 10:10 janbiasi

@janbiasi yooo we did it!! #84 hahaha

HowieG avatar Oct 12 '23 13:10 HowieG

@janbiasi FYI there wasn't a subsequent change to the types for this change. It's simple but need to go through DefinitelyTyped's kinda thorough PR process. I'm just ignoring for now.

https://github.com/DefinitelyTyped/DefinitelyTyped/commit/1709f05c255d8f1c30d115fb216722c39f6a7283

HowieG avatar Oct 12 '23 13:10 HowieG

@HowieG Yea! 😎 Thanks for the support on this one. I think I should have time to go through the definitely typed PR process pretty soon but as you said, it also works without them - I'll keep you updated :)

janbiasi avatar Oct 12 '23 16:10 janbiasi

Alternatively, I think we're happy to add the types here. I don't think there's any real need to use DefinitelyTyped

rschristian avatar Oct 13 '23 01:10 rschristian

Alternatively, I think we're happy to add the types here. I don't think there's any real need to use DefinitelyTyped

Thanks for the hint @rschristian, I just opened a PR https://github.com/preactjs/preact-custom-element/pull/85 for this.

cc @HowieG

janbiasi avatar Oct 15 '23 16:10 janbiasi