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

fix: only externalize @types/ from devDependencies #471

Merged
merged 1 commit into from
Dec 27, 2024
Merged

Conversation

pi0
Copy link
Member

@pi0 pi0 commented Dec 27, 2024

#469 wrongly added all devDependencies as externals which is wrong (they are not shipped). this PR adds only @types/* dev deps as implicit externals which is usually desired.

@pi0 pi0 merged commit 9360293 into main Dec 27, 2024
2 checks passed
@pi0 pi0 deleted the fix/types-dev branch December 27, 2024 23:49
@kricsleo
Copy link
Contributor

kricsleo commented Feb 8, 2025

Sorry, but why are @types/* in devDependencies added as implicit externals? 🤔
I think all devDependencies should not be added, otherwise causes issues like #303 (comment)

After updating to the latest unbuild, the reproduction build outputs:

import { Context } from '/home/projects/github-7jr8pu/node_modules/.pnpm/@types+aws-lambda@8.10.119/node_modules/@types/aws-lambda/index.d.ts';

declare function test(): string;

declare const halo: (e?: any, c?: Context) => any;

export { halo, test };

The Context is not bundled, which is not desirable, I guess.

Removing @types/* from externals correctly bundles the types, right? 🤔

/**
 * {@link Handler} context parameter.
 * See {@link https://docs.aws.amazon.com/lambda/latest/dg/nodejs-prog-model-context.html AWS documentation}.
 */
interface Context {
    callbackWaitsForEmptyEventLoop: boolean;
    functionName: string;
    functionVersion: string;
    invokedFunctionArn: string;
    memoryLimitInMB: string;
    awsRequestId: string;
    logGroupName: string;
    logStreamName: string;
    identity?: CognitoIdentity | undefined;
    clientContext?: ClientContext | undefined;

    getRemainingTimeInMillis(): number;

    // Functions for compatibility with earlier Node.js Runtime v0.10.42
    // No longer documented, so they are deprecated, but they still work
    // as of the 12.x runtime, so they are not removed from the types.

    /** @deprecated Use handler callback or promise result */
    done(error?: Error, result?: any): void;
    /** @deprecated Use handler callback with first argument or reject a promise result */
    fail(error: Error | string): void;
    /** @deprecated Use handler callback with second argument or resolve a promise result */
    succeed(messageOrObject: any): void;
    // Unclear what behavior this is supposed to have, I couldn't find any still extant reference,
    // and it behaves like the above, ignoring the object parameter.
    /** @deprecated Use handler callback or promise result */
    succeed(message: string, object: any): void;
}

interface CognitoIdentity {
    cognitoIdentityId: string;
    cognitoIdentityPoolId: string;
}

interface ClientContext {
    client: ClientContextClient;
    Custom?: any;
    env: ClientContextEnv;
}

interface ClientContextClient {
    installationId: string;
    appTitle: string;
    appVersionName: string;
    appVersionCode: string;
    appPackageName: string;
}

interface ClientContextEnv {
    platformVersion: string;
    platform: string;
    make: string;
    model: string;
    locale: string;
}

declare function test(): string;
declare const halo: (e?: any, c?: Context) => any;

export { halo, test };

@pi0
Copy link
Member Author

pi0 commented Feb 10, 2025

I never add @types/ as normal dependencies nor (re)bundle them. If you need to explicitly bundle something you can use inlineDependencies

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.

2 participants