Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Lib bundled not properly, ts files in dist are not strict typed. #932

Open
WavyWalk opened this issue Jan 27, 2025 · 3 comments
Open

Lib bundled not properly, ts files in dist are not strict typed. #932

WavyWalk opened this issue Jan 27, 2025 · 3 comments
Assignees

Comments

@WavyWalk
Copy link

Hi! 👋

Firstly, thanks for your work on this project! 🙂

Today I used patch-package to patch [email protected] for the project I'm working on.

App is not bundled correctly, and ts files directly imported into my project and compiler runs checks on them.

excluding node_modules and skipLibCheck: true in tsconfig do not resolve the issue.

Lost couple of hours for debugging but can continue after using patch-package to get through

Here is the diff that solved my problem:

diff --git a/node_modules/storyblok-js-client/src/sbFetch.ts b/node_modules/storyblok-js-client/src/sbFetch.ts
index 1be5d4f..842af4c 100644
--- a/node_modules/storyblok-js-client/src/sbFetch.ts
+++ b/node_modules/storyblok-js-client/src/sbFetch.ts
@@ -114,7 +114,7 @@ class SbFetch {
       )}`;
     }
     else {
-      body = JSON.stringify(this.parameters);
+      body = JSON.stringify(this.parameters) as unknown as any;
     }
 
     const url = new URL(urlString);
diff --git a/node_modules/storyblok-js-client/src/sbHelpers.ts b/node_modules/storyblok-js-client/src/sbHelpers.ts
index 75a2262..c600a43 100644
--- a/node_modules/storyblok-js-client/src/sbHelpers.ts
+++ b/node_modules/storyblok-js-client/src/sbHelpers.ts
@@ -78,7 +78,7 @@ export class SbHelpers {
           prefix ? prefix + encodeURIComponent(`[${enkey}]`) : enkey
         }=${encodeURIComponent(value)}`;
       }
-      pairs.push(pair);
+      pairs.push(pair as never);
     }
     return pairs.join('&');
   }

This issue body was partially generated by patch-package.

@edodusi
Copy link
Contributor

edodusi commented Jan 28, 2025

Hi @WavyWalk , thanks for reporting this.

Something I need to understand is if this is a problem with the code or with the build, I'm aware that this needs to be better typed and we are working on a roadmap for the next release with proper types and better dx, but do you think this is an issue that we can fix now working on the build?

@WavyWalk
Copy link
Author

WavyWalk commented Jan 28, 2025

@edodusi
TLDR:

  • quick fix the types directly in code just like in a diff, so it already works for people having problems like we do.
  • long term do not include ts files into the package artifacts.

Longer version:
The general problem is that you include ts files into the package distribution. Ts host projects when importing your package, will try to compile them. Which is not optimal on it's own (among others: compilation time, compatibility, host projects may use different ts settings uncompatible with the code). In our case, if we include strict checks on nulls in our ts config, our code does not compile anymore because compiler as well compiles your package's ts files.

Proper solution would be to not include ts files into the package, so typescript apps instead of using those files use precompiled package code, avoiding these issues altogether and saving on compilation times AND as well on compaitibility.

BUT
For us it would work if you just fix the types in code, so it compiles if it's used. E.g. applying the suggested diffs to make it work with strict ts configs.

This will not require a "breaking change" how package is distributed.

@edodusi edodusi self-assigned this Jan 28, 2025
@edodusi
Copy link
Contributor

edodusi commented Jan 28, 2025

Ok thank you @WavyWalk I will look into this and get back to you 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants