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

Installing @aws-amplify/core adds node globals, which can make TS ignore potential errors #11736

Open
3 tasks done
bogdanailincaipnt opened this issue Aug 5, 2023 · 15 comments
Assignees
Labels
aws-sdk-js bug Something isn't working Core Related to core Amplify issues TypeScript Related to TypeScript issues

Comments

@bogdanailincaipnt
Copy link

Before opening, please confirm:

JavaScript Framework

Not applicable

Amplify APIs

Not applicable

Amplify Categories

Not applicable

Environment information

# Put output below this line
System:
    OS: Linux 5.0 undefined
    CPU: (6) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
    Memory: 0 Bytes / 0 Bytes
    Shell: 1.0 - /bin/jsh
  Binaries:
    Node: 16.20.0 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 9.4.2 - /usr/local/bin/npm
    pnpm: 8.6.10 - /usr/local/bin/pnpm
  npmPackages:
    @aws-amplify/core: ^5.8.0 => 5.8.0 
    @aws-amplify/core/internals/aws-client-utils:  undefined ()
    @aws-amplify/core/internals/aws-client-utils/composers:  undefined ()
    @aws-amplify/core/internals/aws-clients/pinpoint:  undefined ()
    typescript: 5.1.6 => 5.1.6 

Describe the bug

Installing @aws-amplify/core (or one of its dependends) adds NodeJS globals, which can make TS ignore potential errors.

The @aws-amplify/core package has the "@types/node-fetch" dependency in the "dependencies" list in its package.json.
This is causing TS to think that NodeJS globals (eg. process) are available, even though we are developing for a browser environment.

Among NodeJS globals there are also methods like String.at, or Array.at, which are not supported in ES2020 for example. As you can see in the Stackblitz containers linked below, this can cause TS to miss some errors, since it has its target set as ES2020 in tsconfig.json.

Solution: "@types/node-fetch" and any other package related to types, react-native, or other devtools should be included in "devDependencies", instead of "dependencies" (explanation: https://docs.npmjs.com/cli/v9/configuring-npm/package-json#dependencies - "Please do not put test harnesses or transpilers or other "development" time tools in your dependencies object.", "If someone is planning on downloading and using your module in their program, then they probably don't want or need to download and build the external test or documentation framework that you use.").

I think this is important to solve, because there are a lot of users potentially affected by this. Vite for example sets its default target as ES2020 (roughly).

Expected behavior

TS should not be missing errors.

Reproduction steps

  1. Go to https://stackblitz.com/edit/vitejs-vite-krwz8t?file=package.json,src%2Fmain.ts&terminal=dev
  2. Run npx tsc in the terminal
  3. There should be 3 errors, but there are none.
  4. Go here, where @aws-amplify/core is not installed: https://stackblitz.com/edit/vitejs-vite-knsnqf?file=package.json,src%2Fmain.ts&terminal=dev
  5. Run npx tsc in the terminal
  6. You can see that 3 errors are found

You can also hover over process, at in the main.ts file to see how TS sees them (as NodeJS globals, or as missing).

Code Snippet

// Put your code below this line.

Log output

// Put your logs below this line


aws-exports.js

No response

Manual configuration

No response

Additional configuration

No response

Mobile Device

No response

Mobile Operating System

No response

Mobile Browser

No response

Mobile Browser Version

No response

Additional information and screenshots

No response

@bogdanailincaipnt bogdanailincaipnt added the pending-triage Issue is pending triage label Aug 5, 2023
@cwomack cwomack added Core Related to core Amplify issues TypeScript Related to TypeScript issues dependencies Pull requests that update a dependency file and removed pending-triage Issue is pending triage labels Aug 5, 2023
@cwomack cwomack self-assigned this Aug 7, 2023
@cwomack cwomack added the investigating This issue is being investigated label Aug 8, 2023
AllanZhengYP added a commit to AllanZhengYP/amplify-js that referenced this issue Aug 9, 2023
Also remove the runtime dependency of @types/node-fetch because
the new isomorphic-unfetch fixed the missing types and do not
introduce the @types/node dependency. This fixes aws-amplify#11736
@cwomack
Copy link
Member

cwomack commented Aug 9, 2023

Hello, @bogdanailincaipnt 👋 and thank you for opening this issue. We are currently looking into this and potentially addressing it with a fix soon. We'll keep you updated on the progress, but appreciate your detailed findings and reproduction steps!

@cwomack cwomack added bug Something isn't working and removed investigating This issue is being investigated labels Aug 9, 2023
@cwomack
Copy link
Member

cwomack commented Oct 9, 2023

The developer preview for v6 of Amplify has officially been released with improvements to TypeScript support and much more! Please check out our announcement and updated documentation to see what has changed.

This issue should be resolved within the dev preview and upcoming General Availability for Amplify v6, but let us know with a comment if there are further issues.

@bogdanailincaipnt
Copy link
Author

Nice! I tested with "@aws-amplify/core": "6.0.1-next.a1ea0f2.0" and it looks like it's fixed. You can check here. The TS errors are showing when running npx tsc, which is good.

@cwomack
Copy link
Member

cwomack commented Nov 24, 2023

With the release of the latest major version of Amplify (aws-amplify@>6), this issue should now be resolved! Please refer to our release announcement, migration guide, and documentation for more information.

@cwomack cwomack closed this as completed Nov 24, 2023
@bogdanailincaipnt
Copy link
Author

Hello again, sorry to say this, but I found that the issue still occurs when importing from 'aws-amplify'. You can see here: https://stackblitz.com/edit/vitejs-vite-ibxcgw?file=package.json,tsconfig.json,src%2Fmain.ts&terminal=dev.

If you comment out the line that does import 'aws-amplify';, the errors show up. They should show up either way, because I've specified "types": [], in tsconfig.json, so global types from @types/node should not be included, even though I've installed the @types/node package.

My goal is to be able to specify where I want global types from @types/node to be available (eg. only in files that run in a node environment). It is not correct for them to be considered available in files that are run in the browser.

@bogdanailincaipnt
Copy link
Author

It might be caused by one of these lines:

"types": ["node"]

"types": ["jest", "node"],

@cwomack cwomack reopened this Jan 29, 2024
@AllanZhengYP
Copy link
Member

Hi @bogdanailincaipnt

Thank you for providing the detailed Stackblitz reproduction. Techinically the tsconfig.base.json shouldn't impact the output types, they are only useful for the library maintainers. But I see the problem from the sandbox. We'll investigate it and keep you post on this thread.

@AllanZhengYP AllanZhengYP removed their assignment Feb 1, 2024
@ashika112 ashika112 added V5 and removed V5 labels Aug 20, 2024
@ashika112
Copy link
Member

@bogdanailincaipnt Hi, i started looking into this issue and all our reference in Amplify V6 for @type/node is as dev dependencies. To your comment here, i removed the dependencies there and did a tagged release to test it in stackedBlitz and I still see the issue. While I am looking more into it, i am not sure how having a dev dependency could affect this way.

@ashika112
Copy link
Member

Looking through fresh build locally with just aws-amplify as dependency, i dont see any @types/node as well

Screenshot 2024-08-22 at 12 35 29 PM

@bogdanailincaipnt
Copy link
Author

Hello @ashika112, I appreciate you looking into this! I opened the project locally as well, I think I may have found the cause. After runningnpm install locally, open the node_modules folder and search inside it for this text: /// <reference types="node" />. You should find lots of occurences (the ones listed below). Having /// <reference types="node" /> anywhere inside the code is like having "node" inside tsconfig "types". See https://www.typescriptlang.org/docs/handbook/triple-slash-directives.html#-reference-types-.

59 results - 47 files

@aws-sdk/types/dist-types/blob/runtime-blob-types.node.d.ts:
  1: /// <reference types="node" />
  2: /// <reference types="node" />
  3  import { Readable } from "stream";

@smithy/core/dist-types/submodules/cbor/cbor-types.d.ts:
  1: /// <reference types="node" />
  2  export type CborItemType = undefined | boolean | number | bigint | [CborUnstructuredByteStringType, Uint64] | string | CborTagType;

@smithy/core/dist-types/ts3.4/submodules/cbor/cbor-types.d.ts:
  1: /// <reference types="node" />
  2  export type CborItemType = undefined | boolean | number | bigint | [

@smithy/credential-provider-imds/dist-types/remoteProvider/httpRequest.d.ts:
  1: /// <reference types="node" />
  2: /// <reference types="node" />
  3  import { Buffer } from "buffer";

@smithy/credential-provider-imds/dist-types/ts3.4/remoteProvider/httpRequest.d.ts:
  1: /// <reference types="node" />
  2  import { Buffer } from "buffer";

@smithy/eventstream-serde-node/dist-types/EventStreamMarshaller.d.ts:
  1: /// <reference types="node" />
  2  import { Decoder, Encoder, EventStreamMarshaller as IEventStreamMarshaller, Message } from "@smithy/types";

@smithy/eventstream-serde-node/dist-types/utils.d.ts:
  1: /// <reference types="node" />
  2  import { Readable } from "stream";

@smithy/eventstream-serde-node/dist-types/ts3.4/EventStreamMarshaller.d.ts:
  1: /// <reference types="node" />
  2  import { Decoder, Encoder, EventStreamMarshaller as IEventStreamMarshaller, Message } from "@smithy/types";

@smithy/eventstream-serde-node/dist-types/ts3.4/utils.d.ts:
  1: /// <reference types="node" />
  2  import { Readable } from "stream";

@smithy/node-http-handler/dist-types/node-http-handler.d.ts:
  1: /// <reference types="node" />
  2: /// <reference types="node" />
  3  import { HttpHandler, HttpRequest, HttpResponse } from "@smithy/protocol-http";

@smithy/node-http-handler/dist-types/node-http2-connection-manager.d.ts:
  1: /// <reference types="node" />
  2  import { RequestContext } from "@smithy/types";

@smithy/node-http-handler/dist-types/node-http2-connection-pool.d.ts:
  1: /// <reference types="node" />
  2  import { ConnectionPool } from "@smithy/types";

@smithy/node-http-handler/dist-types/readable.mock.d.ts:
  1: /// <reference types="node" />
  2: /// <reference types="node" />
  3  import { Readable, ReadableOptions } from "stream";

@smithy/node-http-handler/dist-types/write-request-body.d.ts:
  1: /// <reference types="node" />
  2: /// <reference types="node" />
  3  import { HttpRequest } from "@smithy/types";

@smithy/node-http-handler/dist-types/stream-collector/collector.d.ts:
  1: /// <reference types="node" />
  2: /// <reference types="node" />
  3  import { Writable } from "stream";

@smithy/node-http-handler/dist-types/stream-collector/readable.mock.d.ts:
  1: /// <reference types="node" />
  2: /// <reference types="node" />
  3  import { Readable, ReadableOptions } from "stream";

@smithy/node-http-handler/dist-types/ts3.4/node-http-handler.d.ts:
  1: /// <reference types="node" />
  2  import { HttpHandler, HttpRequest, HttpResponse } from "@smithy/protocol-http";

@smithy/node-http-handler/dist-types/ts3.4/node-http2-connection-manager.d.ts:
  1: /// <reference types="node" />
  2  import { RequestContext } from "@smithy/types";

@smithy/node-http-handler/dist-types/ts3.4/node-http2-connection-pool.d.ts:
  1: /// <reference types="node" />
  2  import { ConnectionPool } from "@smithy/types";

@smithy/node-http-handler/dist-types/ts3.4/readable.mock.d.ts:
  1: /// <reference types="node" />
  2  import { Readable, ReadableOptions } from "stream";

@smithy/node-http-handler/dist-types/ts3.4/write-request-body.d.ts:
  1: /// <reference types="node" />
  2  import { HttpRequest } from "@smithy/types";

@smithy/node-http-handler/dist-types/ts3.4/stream-collector/collector.d.ts:
  1: /// <reference types="node" />
  2  import { Writable } from "stream";

@smithy/node-http-handler/dist-types/ts3.4/stream-collector/readable.mock.d.ts:
  1: /// <reference types="node" />
  2  import { Readable, ReadableOptions } from "stream";

@smithy/types/dist-types/blob/blob-payload-input-types.d.ts:
  1: /// <reference types="node" />
  2: /// <reference types="node" />
  3  import { Readable } from "stream";

@smithy/types/dist-types/http/httpHandlerInitialization.d.ts:
  1: /// <reference types="node" />
  2: /// <reference types="node" />
  3  import type { Agent as hAgent, AgentOptions as hAgentOptions } from "http";

@smithy/types/dist-types/streaming-payload/streaming-blob-common-types.d.ts:
  1: /// <reference types="node" />
  2  import type { Readable } from "stream";

@smithy/types/dist-types/streaming-payload/streaming-blob-payload-input-types.d.ts:
  1: /// <reference types="node" />
  2: /// <reference types="node" />
  3  import type { Readable } from "stream";

@smithy/types/dist-types/streaming-payload/streaming-blob-payload-output-types.d.ts:
  1: /// <reference types="node" />
  2: /// <reference types="node" />
  3  import type { IncomingMessage } from "http";

@smithy/types/dist-types/transform/client-payload-blob-type-narrow.d.ts:
  1: /// <reference types="node" />
  2: /// <reference types="node" />
  3  import type { IncomingMessage } from "http";

@smithy/types/dist-types/ts3.4/blob/blob-payload-input-types.d.ts:
  1: /// <reference types="node" />
  2  import { Readable } from "stream";

@smithy/types/dist-types/ts3.4/http/httpHandlerInitialization.d.ts:
  1: /// <reference types="node" />
  2  import { Agent as hAgent, AgentOptions as hAgentOptions } from "http";

@smithy/types/dist-types/ts3.4/streaming-payload/streaming-blob-common-types.d.ts:
  1: /// <reference types="node" />
  2  import { Readable } from "stream";

@smithy/types/dist-types/ts3.4/streaming-payload/streaming-blob-payload-input-types.d.ts:
  1: /// <reference types="node" />
  2  import { Readable } from "stream";

@smithy/types/dist-types/ts3.4/streaming-payload/streaming-blob-payload-output-types.d.ts:
  1: /// <reference types="node" />
  2  import { IncomingMessage } from "http";

@smithy/types/dist-types/ts3.4/transform/client-payload-blob-type-narrow.d.ts:
  1: /// <reference types="node" />
  2  import { IncomingMessage } from "http";

@smithy/util-stream/dist-types/getAwsChunkedEncodingStream.d.ts:
  1: /// <reference types="node" />
  2  import { GetAwsChunkedEncodingStream } from "@smithy/types";

@smithy/util-stream/dist-types/splitStream.d.ts:
  1: /// <reference types="node" />
  2  import type { Readable } from "stream";

@smithy/util-stream/dist-types/ts3.4/getAwsChunkedEncodingStream.d.ts:
  1: /// <reference types="node" />
  2  import { GetAwsChunkedEncodingStream } from "@smithy/types";

@smithy/util-stream/dist-types/ts3.4/splitStream.d.ts:
  1: /// <reference types="node" />
  2  import { Readable } from "stream";

undici-types/content-type.d.ts:
  1: /// <reference types="node" />
  2  

undici-types/cookies.d.ts:
  1: /// <reference types="node" />
  2  

undici-types/fetch.d.ts:
  2  // and https://github.com/node-fetch/node-fetch/blob/914ce6be5ec67a8bab63d68510aabf07cb818b6d/index.d.ts (MIT license)
  3: /// <reference types="node" />
  4  

undici-types/file.d.ts:
  1  // Based on https://github.com/octet-stream/form-data/blob/2d0f0dc371517444ce1f22cdde13f51995d0953a/lib/File.ts (MIT)
  2: /// <reference types="node" />
  3  

undici-types/filereader.d.ts:
  1: /// <reference types="node" />
  2  

undici-types/formdata.d.ts:
  1  // Based on https://github.com/octet-stream/form-data/blob/2d0f0dc371517444ce1f22cdde13f51995d0953a/lib/FormData.ts (MIT)
  2: /// <reference types="node" />
  3  

undici-types/patch.d.ts:
  1: /// <reference types="node" />
  2  

undici-types/websocket.d.ts:
  1: /// <reference types="node" />
  2  

@ashika112
Copy link
Member

ashika112 commented Aug 22, 2024

@bogdanailincaipnt Thanks for the quick response and appreciated the deep dive :) the listed packages in the screenshot are all from @aws-sdk packages (repo) which amplify takes dependency on. I think it needs to be addressed by them so we could stop seeing this i guess. I did find one instance in Amplify Predictions, I will try to remove that and test it (I think, it should have been tree-shaken already though)

@ashika112
Copy link
Member

@bogdanailincaipnt Confirming the Predictions is tree shaken out from aws-amplify umbrella package. I would recommend opening an issue with aws-sdk repo on the above. We have a hard dependency on @aws-sdk/types and that seems to be the core of this issue which brings in @types/node.

@ashika112
Copy link
Member

@bogdanailincaipnt can we close this issue in favor of the one opened in aws-sdk since there is not much we can do at our end?

@bogdanailincaipnt
Copy link
Author

@ashika112 I think this issue should be closed only after the aws-amplify package has a new release that uses a version of @aws-sdk/types that has the issue fixed.

@ashika112
Copy link
Member

@bogdanailincaipnt Sorry you are right. Notice we have pinned dependencies on @aws-sdk/types. Will wait on reply on the other ticket. Thanks. Will mark it accordingly.

@ashika112 ashika112 added aws-sdk-js and removed bug Something isn't working dependencies Pull requests that update a dependency file labels Aug 28, 2024
@cwomack cwomack added the bug Something isn't working label Sep 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aws-sdk-js bug Something isn't working Core Related to core Amplify issues TypeScript Related to TypeScript issues
Projects
None yet
Development

No branches or pull requests

4 participants