prateekbh / preact-material-components

preact wrapper for "Material Components for the web"
https://material.preactjs.com
MIT License
553 stars 81 forks source link

Support to Preact X #1301

Closed fjorgemota closed 4 years ago

fjorgemota commented 5 years ago

Hey.

I observed that there's no support to Preact X yet.

Specifically, the typescript compiler emits an error about code using the attributes attribute in the VNode structure, like this example.

I imagine this is related to the fact that in Preact X the VNode shape has changed, and now attributes is just props in the VNode structure. According to Typescript compiler, there's a few other cases where the same error occurs.

So here's the question: What's the plan to support Preact X? Would this package have a version compatible with both versions (assuming this is the unique problem, of course), or we would release a new version supporting only Preact X?

PS: I know that Preact X is still in alpha but I think it's better to prepare for it than await to a stable release...What you think?

cromefire commented 5 years ago

@prateekbh it seems not that big of a change (Read through the breaking changes section), should we put it in 2.0?

cromefire commented 5 years ago

@fjorgemota could you try this against 2.0 (it's in the 2.0 branch of the repo)? (And maybe paste the errors here if there are not to many of them)

cromefire commented 5 years ago

Would this package have a version compatible with both versions

Yes

fjorgemota commented 5 years ago

Hey @cromefire, I didn't tested the version 2.0 here because it seems to not be a unique package anymore and I honestly don't know how to test it here on my project.

But..this shows that the 2.0 branch has the same problem as master branch, and would not be compatible with Preact X in the actual state.

Would this package have a version compatible with both versions

Yes

Well, I think it would be good if we have some helpers (probably in the base package) to manage the differences in the VNode structure, then...specifically the attributes (now props) attribute. Depending on the number of changes involved, of course, as the overhead over users using Preact X directly may grow overtime (with more breaking changes here and there over the Preact 8 version), I think. Just my two cents. :)

johnhaitas commented 5 years ago

@fjorgemota I have an experimental branch where i made the basic vnode shape changes for Preact X ... It seems that preact 10.0.0-alpha.2 has a bug that has already been fixed on the master branch so i had to link the preact package locally. Preact 10.0.0-alpha.3 should fix that though. There are still some problems I noticed. For example, the IconButton class has a on prop that throws an error with devtools because on is expected to be a function rather than a boolean. Here is my diff with preact-material-components master branch

https://github.com/prateekbh/preact-material-components/compare/master...johnhaitas:experimental/preact-10

cromefire commented 5 years ago

@johnhaitas 1.X is in maintenance mode it will only get bug fixes, for something new please develop against 2.0

fjorgemota commented 5 years ago

Hey @johnhaitas! Nice that you started working on the changes related to add support to Preact X to preact-material-components. :D

As @cromefire noted, these changes should be applied against 2.0 to be applied to the next version...And also the support to Preact 8 shouldn't be removed as commented by @cromefire here.

By the way, great that you're using toChildArray from Preact X to get the same array as Preact 8. It's another thing that'll be needed to support Preact X, together with VNode shape changes.

Drapegnik commented 5 years ago

Hi! If there are any updates with this?

prateekbh commented 5 years ago

Hi, we are pretty close to our 2.0 alpha with just tabs remaining we'll fix this post alpha and also send a fix to 1.x next week

Drapegnik commented 4 years ago

with ts:

Screen Shot 2019-09-03 at 17 34 01

node_modules/preact-material-components/Base/MaterialComponent.d.ts:10:105 - error TS2503: Cannot find namespace 'JSX'.

10 export declare type MaterialComponentProps<PropType> = SoftMerge<PropType & IMaterialComponentOwnProps, JSX.HTMLAttributes>;
                                                                                                           ~~~

node_modules/preact-material-components/Base/MaterialComponent.d.ts:25:19 - error TS2314: Generic type 'MDCComponent<FoundationType>' requires 1 type argument(s).

25     MDComponent?: MDCComponent<any, any>;
                     ~~~~~~~~~~~~~~~~~~~~~~

node_modules/preact-material-components/Button/index.d.ts:18:41 - error TS2503: Cannot find namespace 'JSX'.

18 export declare class Button<PropsType = JSX.HTMLAttributes, StateType = {}> extends MaterialComponent<PropsType & IButtonProps, StateType & IButtonState> {

                                           ~~~

node_modules/preact-material-components/Button/index.d.ts:22:40 - error TS2503: Cannot find namespace 'JSX'.

22     protected materialDom(props: any): JSX.Element;
                                          ~~~

node_modules/preact-material-components/Icon/index.d.ts:9:40 - error TS2503: Cannot find namespace 'JSX'.

9     protected materialDom(props: any): JSX.Element;
                                         ~~~

[5:31:38 PM] Found 5 errors. Watching for file changes.
cromefire commented 4 years ago

error TS2503: Cannot find namespace 'JSX'

For the JSX namespace you must have misconfigured something with the preact types. I think you have to configure the react namespace to preact in ts.

Drapegnik commented 4 years ago

1.5.8 • Public • Published 3 months ago

hmm, does 1.5.8 inludes Preact X fix?

@prateekbh, how could I test it?

cromefire commented 4 years ago

1.5.8 • Public • Published 3 months ago

hmm, does 1.5.8 inludes Preact X fix?

The Preact X fix is less than 3 Month old

Drapegnik commented 4 years ago

@cromefire, https://github.com/preactjs/preact/issues/1036#issuecomment-445890450 I already have

    "jsx": "react",
    "jsxFactory": "h",

in my config and it works.

but falls with preact-material-components

Drapegnik commented 4 years ago

The Preact X fix is less than 3 Month old

so, is Preact X Fix published? can it be used?

cromefire commented 4 years ago

Okay it seems like one would have to alias preact.JSX as JSX which we can't do because it would result in incompatibilities with 8.X

Something like globals.d.ts:

import export JSX = preact.JSX;

could do it.

But I did not test it.

But @prateekbh has to release it first

prateekbh commented 4 years ago

i'll release it today

prateekbh commented 4 years ago

sorry moving this release to end of week, just need to be doubly sure before release

prateekbh commented 4 years ago

now finding how to change channel from @next to stable

prateekbh commented 4 years ago

released 1.6.0

Drapegnik commented 4 years ago

I confirm that works with:

declare global {
  // FIXME
  // https://github.com/preactjs/preact/issues/1036#issuecomment-491409239
  // eslint-disable-next-line @typescript-eslint/no-namespace
  namespace JSX {
    type Element = JSXInternal.Element;
    type HTMLAttributes = JSXInternal.HTMLAttributes;
  }
}