knocklabs / javascript

Official JavaScript packages for interacting with Knock
https://knock.app/
MIT License
13 stars 3 forks source link

Could not find declaration file for module 'phoenix' #24

Open jtferns opened 1 year ago

jtferns commented 1 year ago

When running tsc (TS v4.9.5) on my React app (v18.2.0) that pulls in the Knock notification feed (our tsconfig does not have skipLibCheck enabled), I'm running into the following issue:

node_modules/@knocklabs/client/dist/types/api.d.ts:2:24 - error TS7016: Could not find a declaration file for module 'phoenix'. '/my/app/dir/node_modules/phoenix/priv/static/phoenix.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/phoenix` if it exists or add a new declaration (.d.ts) file containing `declare module 'phoenix';`

2 import { Socket } from "phoenix";
                         ~~~~~~~~~

Found 1 error in node_modules/@knocklabs/client/dist/types/api.d.ts:2

I was able to step around the issue by installing @types/phoenix as a dev-dependency, but I don't think that's the right expectation for a notification feed consumer. I think the actual type/lib is coming from the Knock JS Client dependency, but I'm not using that package directly in my app; should I be?

cjbell commented 1 year ago

Thanks for the bug report here. @knocklabs/client is a dep of this lib, and it should install the appropriate types for phoenix. Will flag a ticket now to fix this!

cjbell commented 1 year ago

Hmm ok just took a look and curiously we do already have @types/phoenix as a dev-dep of @knocklabs/client. I guess it should also be a peer dependency of that lib but going to look into this more!

connorlindsey commented 7 months ago

@jtferns Hey, sorry for the delay but wanted to revisit this issue and see if you're still seeing the type error.

Since then, we released a new React components package and the type error should be resolved. We wrote up a migration guide here to help transition from @knocklabs/react-notification-feed to @knocklabs/react: https://docs.knock.app/in-app-ui/react/migrating-from-react-notification-feed

jtferns commented 7 months ago

Thanks @connorlindsey ; unfortunately still encountering the same issue when I migrated to @knocklabs/react v0.1.4.

├─ @knocklabs/react@npm:0.1.4
│  ├─ Instances: 1
│  ├─ Version: 0.1.4
│  │
│  └─ Dependencies
│     ├─ @knocklabs/client@npm:* → npm:0.8.17
│     ├─ @knocklabs/react-core@npm:* → npm:0.1.2
│     ├─ @popperjs/core@npm:^2.11.8 → npm:2.11.8
│     ├─ lodash.debounce@npm:^4.0.8 → npm:4.0.8
│     ├─ react-popper-tooltip@npm:^4.4.2 → npm:4.4.2
│     └─ react-popper@npm:^2.3.0 → npm:2.3.0

Without @types/phoenix added as a dev-dependency, my tsc (TS v5.1.6) run emits the same failure:

node_modules/@knocklabs/client/dist/types/api.d.ts:2:24 - error TS7016: Could not find a declaration file for module 'phoenix'. '/my/app/dir/node_modules/phoenix/priv/static/phoenix.cjs.js' implicitly has an 'any' type.
  Try `npm i --save-dev @types/phoenix` if it exists or add a new declaration (.d.ts) file containing `declare module 'phoenix';`

2 import { Socket } from "phoenix";
                         ~~~~~~~~~

Found 1 error in node_modules/@knocklabs/client/dist/types/api.d.ts:2
connorlindsey commented 7 months ago

@jtferns Thanks for confirming! Will continue to take a look and see what we need to fix. Could you share your tsconfig as well?

jtferns commented 6 months ago

@jtferns Thanks for confirming! Will continue to take a look and see what we need to fix. Could you share your tsconfig as well?

@connorlindsey sorry for the delay, here's our tsconfig:

{
  "compilerOptions": {
    "target": "ES2019",
    "module": "commonjs",
    "lib": ["ESNext", "DOM", "DOM.Iterable"],
    "jsx": "react",
    "sourceMap": true,
    "removeComments": true,
    "strict": true,
    "noImplicitAny": true,
    "strictNullChecks": true,
    "strictBindCallApply": true,
    "noImplicitThis": true,
    "alwaysStrict": true,
    "noUnusedLocals": true,
    "noUnusedParameters": true,
    "noImplicitReturns": true,
    "noFallthroughCasesInSwitch": true,
    "moduleResolution": "node",
    "rootDirs": ["."],
    "typeRoots": ["node_modules/@types", "types-dir"],
    "allowSyntheticDefaultImports": true,
    "esModuleInterop": true,
    "forceConsistentCasingInFileNames": true
  },
  "include": ["app/**/*.ts", "another.ts", "types-dir"],
  "exclude": ["cypress"]
}