hdoro / sanity-plugin-external-files

Work w/ media hosted in external vendors from inside a Sanity.io studio
44 stars 14 forks source link

fix: add react-rx v2.3.1 as a core dependency to support useMemoObservable #16

Open andrre-ls opened 1 month ago

andrre-ls commented 1 month ago

Sanity v3.47.0 upgraded react-rx to v3, which in turn removes the useMemoObservable hook, required for this plugin to work.

Including v2 of react-rx as a dependency allows the plugin to continue using the now-removed useMemoObservable hook.

(also: 25030eaa475d9ebe42121a98bfde9c2e522f98e0 adds type any to firebase, digital-ocean, and aws plugin variables in order for the build command to work)

hdoro commented 1 month ago

Obrigado pelo trabalho, André!

I'm not 100% on the any type, it'll break the consumption of types when enabling the plugins in users' sanity.config. Can you elaborate more on why this happens and consider splitting your PR into 2 separate ones, one for fixing react-rx and the other for modifying TS types?

andrre-ls commented 1 month ago

I'll preface this by saying that I'm not that well-versed in typescript. I added it because I was getting theses errors when I tried running the build command:

sanity-plugin-digital-ocean-files:build: src/index.tsx:17:14 - error TS2742: The inferred type of 'digitalOceanFiles' cannot be named without a reference to '../../core/node_modules/sanity/lib'. This is likely not portable. A type annotation is necessary.
sanity-plugin-digital-ocean-files:build: 
sanity-plugin-digital-ocean-files:build: 17 export const digitalOceanFiles = definePlugin((userConfig?: UserConfig) => {
sanity-plugin-digital-ocean-files:build:                 ~~~~~~~~~~~~~~~~~
sanity-plugin-digital-ocean-files:build: 
sanity-plugin-digital-ocean-files:build: 
sanity-plugin-digital-ocean-files:build: Found 1 error in src/index.tsx:17
sanity-plugin-digital-ocean-files:build: 
sanity-plugin-s3-files:build: src/index.tsx:17:14 - error TS2742: The inferred type of 's3Files' cannot be named without a reference to '../../core/node_modules/sanity/lib'. This is likely not portable. A type annotation is necessary.
sanity-plugin-s3-files:build: 
sanity-plugin-s3-files:build: 17 export const s3Files = definePlugin((userConfig?: UserConfig) => {
sanity-plugin-s3-files:build:                 ~~~~~~~
sanity-plugin-s3-files:build: 
sanity-plugin-s3-files:build: 
sanity-plugin-s3-files:build: Found 1 error in src/index.tsx:17
sanity-plugin-s3-files:build: 
sanity-plugin-firebase-files:build: src/index.tsx:17:14 - error TS2742: The inferred type of 'firebaseFiles' cannot be named without a reference to '../../core/node_modules/sanity/lib'. This is likely not portable. A type annotation is necessary.
sanity-plugin-firebase-files:build: 
sanity-plugin-firebase-files:build: 17 export const firebaseFiles = definePlugin((userConfig?: UserConfig) => {
sanity-plugin-firebase-files:build:                 ~~~~~~~~~~~~~
sanity-plugin-firebase-files:build: 
sanity-plugin-firebase-files:build: 
sanity-plugin-firebase-files:build: Found 1 error in src/index.tsx:17
sanity-plugin-firebase-files:build: 
sanity-plugin-digital-ocean-files:build:  ELIFECYCLE  Command failed with exit code 1.
sanity-plugin-s3-files:build:  ELIFECYCLE  Command failed with exit code 1.
sanity-plugin-firebase-files:build:  ELIFECYCLE  Command failed with exit code 1.
sanity-plugin-digital-ocean-files:build: ERROR: command finished with error: command (/Users/andre/Sites/buro/sanity-plugin-external-files/packages/digital-ocean) /Users/andre/Library/pnpm/pnpm run build exited (1)
sanity-plugin-digital-ocean-files#build: command (/Users/andre/Sites/buro/sanity-plugin-external-files/packages/digital-ocean) /Users/andre/Library/pnpm/pnpm run build exited (1)

Do you have any input into why this is happening, or how it could be properly fixed?

hdoro commented 1 month ago

This is an issue with how your project is consuming this package's TS definitions. I don't have enough understanding of build processes and output formats to help.

So we can go ahead with this PR, please remove this modification and let's go with only the react-rx part - I'm doing this voluntarily out of respect and consideration for Büro, and don't really have the desire or interest in going deep into TS issues or review thorough PRs 😬

andrre-ls commented 1 month ago

Ok, got it. Removed the TS-related changes. Thank you!