ionic-team / stencil-ds-output-targets

These are output targets that can be added to Stencil for React and Angular.
https://stenciljs.com
MIT License
250 stars 118 forks source link

bug: React 18 createOverlayComponent does not match updated children type signature #259

Closed effinsky closed 1 month ago

effinsky commented 2 years ago

Setup:

"@stencil/core": "^2.15.1" "@stencil/react-output-target": "^0.3.1"

setup directly following https://ionicframework.com/blog/building-react-and-angular-component-libraries-with-stencil-and-nx/

Issue:

In the React lib generated by Stencil, we get a file named createOverlayComponent.tsx that has the following error: Screenshot 2022-05-02 at 12 55 13 Screenshot 2022-05-02 at 12 55 28

Every time we change component code in core Stencil lib, we have the React lib regenerated with the error in place.

Desired behavior:

To have the error not occur in the first place or have a workaround to tweak config so as to have the error ignored.

sean-perkins commented 2 years ago

Thanks for reporting this issue! Are you by chance using React 18? I believe the type signature for children was a breaking change in that release and we have not updated this library to support React 18 yet.

effinsky commented 2 years ago

indeed, React 18. Should have mentioned it, now that I think of it. Any suggestions on how to proceed? And thanks for looking into it!

sean-perkins commented 2 years ago

Thanks for following up! I am not aware of any changes you could make in your project to work around this warning (outside of disabling eslint rules globally, which I do not recommend).

This just needs to be fixed in this project and a new release pushed out. I'll categories this as a bug with our React implementation. I will need to do some discovery on what backwards compatibility looks like for React 18 -> React 17. This may require a new major version of the react output targets to properly support it.

effinsky commented 2 years ago

hey thanks a bunch. we'll keep following up on this.

would you have any idea when such a new major version would be released?

raulfdm commented 2 years ago

Oh, nice this problem is raised already. Looking forward to have this fixed and upgrade my app's react to 18 👏

tfrijsewijk commented 2 years ago

patch-package to the rescue 🚀

In /node_modules/@stencil/react-output-target/react-component-lib/createOverlayComponent.ts change line 155 from:

return ReactDOM.createPortal(this.props.isOpen || isDismissing ? this.props.children : null, this.el);

to:

return <>{ReactDOM.createPortal(this.props.isOpen || isDismissing ? this.props.children : null, this.el)}</>;

Use patch-package to keep the changes after running npm install. I haven't done in-depth testing but it seems to work prety well 🤷‍♂️

risto1913 commented 2 years ago

We are having the same issue at the office. Hoping for a stencil update.

klh commented 2 years ago

wouldn't React.Component<React.PropsWithChildren> be a better fix than returning a fragment here?

klh commented 2 years ago

class Overlay extends Component<PropsWithChildren<Props>> { overlay?: OverlayType; el: HTMLDivElement;

klh commented 2 years ago

We've updated our stencil fork here: https://github.com/duetds/stencil-ds-plugins/tree/bug/react-18-compatibility

and version 0.0.19 of our fork fixes the react issue.

{
...
"@duetds/stencil-react-output-target" : "0.0.19",
...
}
theo-staizen commented 2 years ago

@klh does your fork have any other changes compared to main package?

klh commented 2 years ago

some, we've fixed some issues with event naming and cleaning after mounting them, and upgraded the actual output targets to latest stencil and newer typescript - best you look through it yourself :)

I tried valiantly to move to Angular 14 as well - but ran into multiple issues that where insurmountable without a complete rewrite

HOWEVER I think this whole output thing should be rethought slightly - angular 14 supports web components out of the box, as does React 19 (on paper) - so perhaps this outputting to various targets should just not be done... We're looking into just dropping the headache and maintenance of this fork.

and telling people to use the web components as is.

christian-bromann commented 1 month ago

This issue should be resolved with the latest @stencil/react-output-target version. If it still appears, please let me know and I can re-open the issue. Thanks!

WilmerCo commented 1 month ago

This issue should be resolved with the latest @stencil/react-output-target version. If it still appears, please let me know and I can re-open the issue. Thanks!

Hi, I'm still facing the issue: `src/react-component-lib/createOverlayComponent.tsx:140:13 - error TS2322: Type 'PropsWithoutRef & { forwardedRef: ForwardedRef; }' is not assignable to type 'IntrinsicAttributes & IntrinsicClassAttributes & Readonly'. Type 'PropsWithoutRef & { forwardedRef: ForwardedRef; }' is not assignable to type 'Readonly'.

140 return <Overlay {...props} forwardedRef={ref} />;`

this is the package.json of the components library:

{
  "name": "my-components",
  "version": "1.26.2-beta",
  "description": "-",
  "main": "dist/index.cjs.js",
  "module": "dist/index.js",
  "es2015": "dist/esm/index.js",
  "es2017": "dist/esm/index.js",
  "types": "dist/types/index.d.ts",
  "collection": "dist/collection/collection-manifest.json",
  "collection:main": "dist/collection/index.js",
  "unpkg": "dist/my-components/my-components.esm.js",
  "repository": {},
  "files": [
    "dist/",
    "loader/"
  ],
  "scripts": {
    "stencil": "stencil build --prod --watch --serve --docs",
    "build": "stencil build --docs",
    "build:prod": "stencil build --prod",
    "build:prod-watch": "stencil build --prod --watch",
    "start": "npm-run-all -p stencil storybook",
    "test": "stencil test --spec --e2e",
    "test:unit": "stencil test --spec",
    "test.watch": "stencil test --spec --e2e --watchAll",
    "test:unit-coverage": "stencil test --spec --max-workers=8 --coverage --no-cache",
    "generate": "stencil generate",
    "post-build": "node src/scripts/postbuild.js",
    "storybook": "storybook dev -p 6006",
    "build-storybook": "storybook build"
  },
  "dependencies": {
    "@stencil/core": "^4.22.1",
    "my-basic-components": "1.69.0"
  },
  "devDependencies": {
    "@babel/core": "^7.25.2",
    "@babel/preset-env": "^7.25.3",
    "@babel/preset-react": "^7.24.7",
    "@babel/preset-typescript": "^7.24.7",
    "@stencil/react-output-target": "0.7.3",
    "@stencil/sass": "^3.0.12",
    "@storybook/addon-a11y": "^8.2.9",
    "@storybook/addon-docs": "^8.2.9",
    "@storybook/addon-essentials": "^8.2.9",
    "@storybook/addon-interactions": "^8.2.9",
    "@storybook/addon-links": "^8.2.9",
    "@storybook/addon-themes": "^8.2.9",
    "@storybook/addon-webpack5-compiler-babel": "^3.0.3",
    "@storybook/blocks": "^8.2.9",
    "@storybook/docs-mdx": "^3.1.0",
    "@storybook/html": "^8.2.9",
    "@storybook/html-webpack5": "^8.2.9",
    "@storybook/test": "^8.2.9",
    "@types/babel__core": "^7.20.5",
    "@types/jest": "^29.5.12",
    "@types/node": "^18.19.42",
    "jest": "^29.7.0",
    "jest-cli": "^29.7.0",
    "npm-run-all": "^4.1.5",
    "prettier": "3.3.3",
    "puppeteer": "23.1.0",
    "remark-gfm": "^4.0.0",
    "spdx-satisfies": "^5.0.1",
    "storybook": "^8.2.9"
  },
  "license": "MIT"
}

stencil.config.ts:

import { Config } from "@stencil/core";
import { reactOutputTarget } from "@stencil/react-output-target";
import { sass } from "@stencil/sass";

export const config: Config = {
    namespace: "my-components",
    globalStyle: "./src/styles/styles.scss",
    globalScript: "./src/scripts/app.ts",
    outputTargets: [
        reactOutputTarget({
            outDir: '../my-components-react/src',
        }),
        {
            type: "dist",
            esmLoaderPath: "../loader"
        },
        {
            type: 'dist-custom-elements',
            externalRuntime: false,
            dir: 'components'
        },
        {
            type: "docs-readme"
        },
        {
            type: "www",
            serviceWorker: null // disable service workers
        }
    ],
    testing: {
        browserHeadless: "new"
    },
};

tsConfig:

{
  "compilerOptions": {
    "skipLibCheck": true,
    "allowSyntheticDefaultImports": true,
    "allowUnreachableCode": false,
    "declaration": false,
    "experimentalDecorators": true,
    "lib": [
      "dom",
      "es2017"
    ],
    "moduleResolution": "node",
    "module": "esnext",
    "target": "es2017",
    "noUnusedLocals": true,
    "noUnusedParameters": true,
    "jsx": "react",
    "jsxFactory": "h"
  },
  "include": [
    "src"
  ],
  "exclude": [
    "node_modules",
    "stencil.config.ts"
  ]
}

and this is the package.json in the react:

{
    "name": "my-components-react",
    "sideEffects": false,
    "version": "1.26.2-beta",
    "description": "-",
    "repository": {},
    "author": "",
    "license": "MIT",
    "scripts": {
        "build": "yarn install && yarn run clean && yarn run compile",
        "clean": "rm -rf dist",
        "compile": "yarn run tsc",
        "tsc": "tsc -p ."
    },
    "main": "./dist/index.js",
    "module": "./dist/index.js",
    "types": "./dist/index.d.ts",
    "files": [
        "dist/"
    ],
    "devDependencies": {
        "@types/mocha": "^10.0.9",
        "@types/react": "^18",
        "@types/react-dom": "^18",
        "@typescript-eslint/eslint-plugin": "^8.8.1",
        "@typescript-eslint/parser": "^8.8.1",
        "@vitejs/plugin-react": "^4.3.2",
        "@wdio/cli": "^9.1.5",
        "@wdio/globals": "^9.1.5",
        "@wdio/mocha-framework": "^9.1.3",
        "@wdio/spec-reporter": "^9.1.3",
        "eslint": "^8.57.0",
        "eslint-plugin-react-hooks": "^4.6.2",
        "eslint-plugin-react-refresh": "^0.4.12",
        "expect-webdriverio": "^5.0.3",
        "react": "^18.3.1",
        "tsx": "^4.19.1",
        "typescript": "^5.6.3",
        "vite": "^5.4.8",
        "wdio-vite-service": "^1.0.9",
        "webdriverio": "^9.1.5"
    },
    "dependencies": {
        "@stencil/react-output-target": "0.7.3",
        "my-components": "1.26.2-beta",
        "react-dom": "^18.3.1"
    },
    "peerDependencies": {
        "my-components": "1.26.2-beta",
        "react": "^18.3.1",
        "react-dom": "^18.3.1"
    },
    "jest": {
        "preset": "ts-jest",
        "setupTestFrameworkScriptFile": "<rootDir>/jest.setup.js",
        "testPathIgnorePatterns": [
            "node_modules",
            "dist"
        ]
    }
}

tsconfig:

{
    "compilerOptions": {
        "target": "ES2020",
        "useDefineForClassFields": true,
        "lib": ["ES2020", "DOM", "DOM.Iterable"],
        "module": "ESNext",
        "skipLibCheck": true,

        /* Bundler mode */
        "moduleResolution": "bundler",
        "resolveJsonModule": true,
        "isolatedModules": true,
        "noEmit": true,
        "jsx": "react-jsx",

        /* Linting */
        "strict": true,
        "noFallthroughCasesInSwitch": true,
        "skipDefaultLibCheck": true,
        "allowUnreachableCode": false,
        "allowSyntheticDefaultImports": true,
        "declaration": true,
        "emitDecoratorMetadata": true,
        "experimentalDecorators": true,
        "esModuleInterop": true,
        "noImplicitAny": true,
        "noImplicitReturns": true,
        "noUnusedLocals": true,
        "noUnusedParameters": true,
        "outDir": "dist",
        "removeComments": false,
        "sourceMap": false,
        "typeRoots": ["../../node_modules/@types"],
        "types": ["node", "jest","react","react-dom","babel__core"]
    },
    "include": ["src/**/*.ts", "src/**/*.tsx"],
    "exclude": ["**/__tests__/**"],
    "compileOnSave": false,
    "buildOnSave": false
}
christian-bromann commented 1 month ago

@WilmerCo can you maybe create a small GitHub project for me to check out?

ionitron-bot[bot] commented 1 week ago

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of the output targets, please create a new issue and ensure the template is fully filled out.