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

💥 Type exports for ContextT, LogDNABrowserOptionsT, ILogDNABrowserLogger (and others) broken #7

Closed
prescience-data opened this issue May 9, 2021 · 6 comments

Comments

@prescience-data
Copy link

Problem

Cannot get visibility on exported types.

Cause

I believe something is getting mangled during Rollup.

This is your dist/types/index.d.ts file, which appears to circularly reference itself on line 1, and does not export the types it is looking for.

However, those types are exported from src/index.d.ts (as seen in the screenshot), thus my inference that something must be occurring during Rollup.

import { ContextT, LogDNABrowserOptionsT, ILogDNABrowserLogger } from './index.d';
declare class LogDNABrowserLogger implements ILogDNABrowserLogger {
    Plugins: {
        PerformanceMeasurePlugin: typeof import("./plugins/performance-measure").default;
    };
    private context;
    private logger?;
    private sessionManager;
    private sampleRateScore;
    private staticContext;
    private plugins;
    private options;
    init(ingestionKey: string, options?: LogDNABrowserOptionsT): LogDNABrowserLogger;
    addContext(context: ContextT): LogDNABrowserLogger;
    setSessionId(sessionId: string): LogDNABrowserLogger;
    clearContext(): void;
    error(message: any, lineContext?: object): void;
    warn(message: any, lineContext?: object): void;
    debug(message: any, lineContext?: object): void;
    info(message: any, lineContext?: object): void;
    log(message: any, lineContext?: object): void;
    captureError(error: Error | ErrorEvent, lineContext?: object): void;
    registerMethod(name: string, fn: Function): void;
    private formatMessageForDebug;
    private registerDefaultPlugins;
    private registerPlugin;
    private registerPlugins;
    private logLines;
    private getStackTrace;
    /**
     * Fetches information about the context that is unlikely to change during the session (ex. browser)
     */
    private getStaticContext;
    /**
     * Fetches information about the context that is likely to change during the session (ex. window location)
     */
    private getDynamicContext;
}
declare const _default: LogDNABrowserLogger;
export default _default;

Error

TS2614: Module '"../../node_modules/@logdna/browser/dist/types"' has no exported member 'LogDNABrowserOptionsT'.

Screenshot

image

@TerryMooreII
Copy link
Collaborator

@prescience-data Thank you for reporting this issue. I will take a look to see what is going on and get back to you.

@TerryMooreII
Copy link
Collaborator

@prescience-data Typescript support has been fixed in the latest release v1.1.1

@prescience-data
Copy link
Author

Hi @TerryMooreII

Unfortunately, this does still not have the exports provided.

image

This is the current dist export on 1.1.1:
image

image

I can do a PR if you like?

@michaelshi-logdna
Copy link
Contributor

hey @prescience-data really sorry to hear that it's still not working. I've been digging with @TerryMooreII a bit looking at the issue and we can't seem to reproduce the missing default type that it shows in your IDE currently.

On 1.1.1 we're seeing the default property being exported properly from TS, and we're not sure how to reproduce the issue you're running into. (Currently running TS version 4.2.4 within VS Code, this is a Svelte example TS project installed via yarn, but tested with create-react-app as well)

image

I'm wondering if it's possible there's something poorly cached in your TS server and if you've tried restarting the TS server, or if there's an issue in lower versions of TS.

Could you try restarting your TS server and see if that resolves the issue, or share what version of TS you're currently running?

If you know/see an issue in our rollup config that's causing the types to go in the wrong place as well, would appreciate if you can point where you're seeing the issue and we can try addressing that as well.

@prescience-data
Copy link
Author

I pushed a PR you can check out that shows the issue 🎯🚀

@prescience-data
Copy link
Author

prescience-data commented May 21, 2021

@michaelshi-logdna

Essentially, I would generally expect to be able to destructure all the types out of the export as:

import type { ILogDNABrowserLogger, LogDNABrowserOptionsT, LogType } from "@logdna/browser"

Currently, as per your screenshot above, the only export available is the concrete LogDNABrowserLogger class, and despite using classes as a type annotation being anti-pattern (as opposed to the interface), it unfortunately still doesn't provide any of the other types such as the Options and LogType etc 😞).

The exports in #13 provide all application types to the developer/user.

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

3 participants