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

Types: Fix importing non-default types #185

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

orgads
Copy link
Contributor

@orgads orgads commented Oct 31, 2024

Fixes #181.

@orgads
Copy link
Contributor Author

orgads commented Oct 31, 2024

Please notice I created 3 separate commits.

  • Remove declare module and reformat the file with 2 spaces.
  • Fix missing or wrong types.
  • Fix the export issue.

@orgads
Copy link
Contributor Author

orgads commented Oct 31, 2024

@orgads
Copy link
Contributor Author

orgads commented Oct 31, 2024

Wait. This works with ESM, but breaks CJS :(

I'll try to fix this.

Forget it, it works fine in CJS, and in TS/CJS it works if you enable esModuleInterop.

@orgads orgads marked this pull request as ready for review October 31, 2024 13:29
@danvirsen
Copy link
Contributor

danvirsen commented Oct 31, 2024

After a quick test this seems to work fine. At least my project isn't complaining when I switch from my PR to this 🙂.

Last week I did however run into an issue with the types "parseUri" function, so if you could update that as well I think that I can close my PR in favour of yours.

I use the following locally, but I haven't had the energy to check if any values could be null or something else in some cases.

export function parseUri(uri: string): {
    scheme: string,
    user: string;
    password: string,
    host: string;
    port: number,
    params?: Record<string, any>;
    headers?: Record<string, string>;
};

Edit: I just checked the source code, and parseUri doesn't account for "tel:"-addresses.

I suppose this would be more correct:

export function parseUri(uri: string): {
    family?: string,
    scheme: string,
    user?: string | undefined;
    password?: string | undefined,
    host?: string;
    port?: number;
    number?: string;
    context?: string | null;
    params: Record<string, any>;
    headers?: Record<string, string>;
  };

It's not pretty but I believe it's accurate.

@orgads
Copy link
Contributor Author

orgads commented Oct 31, 2024

Fixed, thanks for letting me know.

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

Successfully merging this pull request may close these issues.

darchtio-srf types are not correctly exported using TS
2 participants