diff with master#2
Draft
kirillgroshkov wants to merge 15 commits into
Draft
Conversation
# Conflicts: # lib/file_transfer_agent/s3_util.js # package.json
# Conflicts: # index.d.ts # lib/file_transfer_agent/s3_util.js # package.json
to avoid peerDependency warning
Major sync from upstream (274 commits, 1.15.0 -> 2.4.1): - TypeScript build (lib/*.ts compiled to dist/), main now ./dist/index.js - Native minicore (NAPI/Rust) bundled binaries - Removed browser build (lib/browser.js, lib/http/browser.js, lib/logger/browser.js) - Removed samples/ - ESLint replaced with oxlint + prettier - New auth: workload identity, PAT, OAuth authorization code/client credentials - Wiremock-based test harness - Drops Node 14/16/17 (engines: >=18) Conflict resolutions preserving the fork: - package.json: kept @naturalcycles/snowflake-sdk name, bumped to 2.4.1. Moved heavy cloud SDKs (@aws-sdk/*, @smithy/*, @aws-crypto/sha256-js, @azure/storage-blob, @azure/identity, google-auth-library) from dependencies to optional peerDependencies. Dropped @google-cloud/storage (upstream replaced it with google-auth-library). - index.d.ts: kept module name '@naturalcycles/snowflake-sdk'. - README.md: kept fork preamble; npm badge + install line use scoped name. - .gitignore, s3_util.js: additive merge. Follow-up needed: verify lazy-require gates across lib/* (file_transfer_agent, authentication/auth_workload_identity, etc.) so optional peers can actually stay uninstalled. Update CLAUDE.md to reflect v2.x architecture.
…ocally These packages are also declared as optional peerDependencies (the fork's slim-install pattern). Listing them in devDependencies as well means: - Local dev / CI: `npm install` pulls them in, so TypeScript compilation resolves their types and tests that exercise S3/Azure/GCP code paths can run. - Consumers installing @naturalcycles/snowflake-sdk: they remain optional peers — npm does not install them unless the consumer opts in. Also pin agent-base ^7.1.0 as a direct dependency to override the older 6.x copy that axios's transitive https-proxy-agent@5 hoists to the top level, which broke `lib/agent/https_proxy_agent.ts` (needs the v7 API).
Reflect the new reality after merging upstream v2.4.1: - Browser build is gone; TypeScript build produces dist/. - ESLint replaced with oxlint + prettier; husky/lint-staged pre-commit. - New auth: workload identity (AWS/Azure/GCP), PAT, OAuth authorization code/client credentials, auth coordinator, SPCS tokens. - New lib/minicore (NAPI Rust) with prebuilt binaries. - New lib/agent/crl_validator (CRL revocation). - New lib/telemetry, lib/disk_cache, lib/proxy_util. - Wiremock test harness for integration tests. - Node engine raised to >=18. Also document the fork-maintenance rules so the next upstream sync is less surprising: - The full peerDep set we maintain, and why each package is in devDependencies as well (build/test needs types). - The lazy-require discipline cloud-SDK callsites must follow so the optional peers can actually stay uninstalled at consumer install time. - The single divergence point (`declare module '@naturalcycles/snowflake-sdk'` in index.d.ts).
…them
Without these changes, the fork's slim-install promise is broken: simply
calling `require('@naturalcycles/snowflake-sdk')` (or `require('./dist')`)
crashes with MODULE_NOT_FOUND unless every cloud SDK is installed,
because upstream v2.x added several eager-load chains:
1. lib/services/sf.js (loaded on every connection) eagerly requires
lib/telemetry/platform_detection.ts, which static-imports
@aws-sdk/client-sts.
2. lib/authentication/authentication.js eagerly requires
auth_workload_identity.ts, which static-imports all three
attestation_*.ts files, each of which static-imports its respective
cloud SDK (@aws-sdk/*, @smithy/*, @aws-crypto/sha256-js, @azure/identity,
google-auth-library).
3. lib/file_transfer_agent/remote_storage_util.js (loaded by every
Statement) eagerly requires s3_util.js and azure_util.js, which had
top-level `require('@smithy/node-http-handler')` and
`require('@azure/storage-blob')`.
Convert each static import to a `require()` inside the function that
actually uses the SDK. TypeScript type information is preserved via
`import type { ... }` so .d.ts output and IDE autocomplete still work.
Verified by stashing every optional peer out of node_modules and
calling `sdk.createConnection({...})` + force-loading the
auth_workload_identity and platform_detection modules — all succeed.
Test suite still reports the same 1001 passing / 11 failing baseline
(failures are pre-existing infrastructure dependencies on
hang_webserver.py and fixture files).
Now that the audit is done, replace the forward-looking warning with the actual rule: name the files that follow the pattern, show the TypeScript snippet, and provide the verification one-liner for future upstream syncs.
When this was added (commit f1ae799), the cloud-SDK devDependencies hadn't been added yet, so npm's top-level resolution of `agent-base` landed on 6.0.2 (transitive via axios -> https-proxy-agent@5), and `tsc` couldn't find the v7-only `AgentConnectOpts` type that lib/agent/https_proxy_agent.ts imports. After adding the cloud SDKs to devDependencies, @azure/identity -> @azure/core-rest-pipeline -> @typespec/ts-http-runtime -> http-proxy-agent@7 pulls agent-base@7.1.x to the top level on its own, so the explicit dep is redundant for the build. At runtime our direct dep on https-proxy-agent@7 ensures the v7 API is used regardless. Removing brings us closer to upstream's package.json (less divergence to maintain on future syncs). Verified: clean install yields 712 packages instead of 713; tsc passes; tests still report 1001/11.
Local TypeScript build still emits sourcemaps (useful for in-tree debugging via ts-node and for stepping through dist/ during local investigation), but consumers don't benefit from them. Excluding removes 108 files (~600 KB unpacked, ~100 KB packed) from the published @naturalcycles/snowflake-sdk tarball. dist/test/ was already correctly excluded by the existing `dist/test/**/*` rule; verified via `npm pack --dry-run`.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR to track changes with upstream.
Not to be merged.