-
Notifications
You must be signed in to change notification settings - Fork 82
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
[$250 bounty] Types breaking with TS 5.1+ ✅ #297
Comments
Looks like 5.2 breaks as well. |
I will pay $200 for anyone to fix this. |
@nandorojo Could you please elaborate what error messages you had and how you managed to encounter them?
To be honest I tried updating TS and couldn't reproduce any issue. One of the reasons could be that it was impossible to run the example apps as the Expo SDK version (v40) was too low and not supported. If you want I can migrate all packages to the most recent versions - not just TS. Related issues, pull requests and other sources: |
@403access Just run |
Hey thanks for investigating this! Let me post a better update of the issue I'm facing. The problem isn't in the Dripsy repo. Rather, if I use Dripsy in a separate repo that upgrades to 5.2, then I don't get TS declaration merging working properly. I haven't even tried to reproduce this outside of the BeatGig repo, but I'll do this for you shortly. |
Szia* Tamás (@TamasSzigeti), thanks for exposing that command. I would expect such a command to be located in the (root level) package.json. Your advice is guiding me in the right direction, thank you 🙏 I still didn't understand how everything can be bundled without having expo-linear-gradient listed in the corresponding From my perspective it is not only updating TS that needs to be tackled. Fernando said that
*szia = hello in Hungarian language for everybody wondering ;) |
Okay, thanks. Maybe it's a topic for another issue: Do you plan to upgrade the dependencies Dripsy uses, e.g. Expo SDK etc.? Thanks! |
Yeah! The example app itself is pretty outdated, I haven’t really used it for testing features in a long time, only testing types. Since we’re feature stable for now the only thing to test there I think would be TS. Ideally the repo would upgrade to 5.2 as well, but I would say this is a secondary concern to me at the moment. The biggest thing I want to fix is letting an app on TS 5.2 use dripsy. The best way to investigate this might be to create a new app, install Dripsy, set it up, upgrade to TS 5.2, and see if it works. And if it doesn’t, perhaps editing Dripsy types in node_modules to see why it isn’t working properly. It might even be a one line change. |
@nandorojo The following repository contains dedicated branches for the versions
Couldn't reproduce. The repository is a pure Expo project. Should I have created one with Solito? Maybe it would be better waiting for your input. Invested quite some time already. Thanks! |
Interesting, let me reproduce better on my end and get back to you. |
@403access your repro appears to be missing the TypeScript setup which requires using declaration merging: const theme = makeTheme({
// your theme
})
type MyTheme = typeof theme
declare module 'dripsy' {
// eslint-disable-next-line @typescript-eslint/no-empty-interface
interface DripsyCustomTheme extends MyTheme {}
} The result of |
To say again what the issue is:
Do you get autocomplete for theme values in the Raised to $250. |
The repro: adding |
Created a PR with the fix: #304 |
Bounty claimed by @Elie-A-98 |
Thank you all for your help! |
Sigh, have to find out why 5.1 isn't working.
5.0.4
works fine for meThe text was updated successfully, but these errors were encountered: