Upload data api three layer refactor#14798
Open
osama-rizk wants to merge 22 commits intomainfrom
Open
Conversation
🦋 Changeset detectedLatest commit: 46eacc1 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
9726d21 to
97d6321
Compare
* fix: update change set config. * add change set back
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
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.
Description of changes
Refactor the internal
uploadDataAPI in@aws-amplify/storageto follow thethree-layer architecture (foundation / client / server):
foundation/— environment-agnostic core. ContainscalculateContentCRC32and
calculateContentMd5, which now receive aFoundationContext(
{ amplify, readFile, toBase64 }) via dependency injection instead ofbranching on the runtime.
client/— browser + React Native implementations ofreadFileandtoBase64(with.native.tssplits preserved), plus the publicclient-side
uploadDataAPI.server/— Node.js implementations and the public server-sideuploadDataAPI, which injects the request-scoped Amplify instance fromgetAmplifyServerContext.The
uploadDatacall graph (putObjectJob,getMultipartUploadHandlers,uploadPartExecutor,loadOrCreateMultipartUpload) now threadsFoundationContextinstead ofAmplifyClassV6, so the foundation layer isfully free of environment-discriminating logic.
Public API surface is unchanged. Routing to the correct bundle per
environment is handled via conditional exports in
package.json(
browser/import/require), and the existingaws-amplify/storage/serversubpath is preserved for backward compatibility.Legacy
providers/s3/utils/{crc32,md5,readFile}.tsare kept as shims fordeleteObjectsuntilremoveis migrated; new code imports fromfoundation/utilsandclient|server/utilsinstead.This is the first API migrated to the three-layer pattern; follow-up PRs
will apply the same split to the remaining Storage APIs.
Issue #, if available
N/A — internal refactor, no customer-facing behavior change.
Description of how you validated changes
uploadDataupdated to passFoundationContextinstead of
Amplify; all continue to pass.foundation/utils/*,client/utils/*, andserver/utils/*modules. New code has 100%statement/function/line coverage.
aws-amplifyJest config now runs two projects so the umbrellapackage's exports are verified under both resolution paths:
nodeproject (pinnedcustomExportConditions: ['node']) — existingtests, asserts the SSR surface of
@aws-amplify/storageinexports.test.ts.browserproject (jsdom defaults) — newexports.browser.test.tsasserts the full client-side surface (
uploadData,downloadData,remove,list,getProperties,copy,getUrl,isCancelError,StorageError,DEFAULT_PART_SIZE).babel-jestwith@babel/preset-env(modules: 'commonjs') is usedto transform the
.mjsESM bundle emitted by@aws-amplify/storageso Jest can load it under the browser condition; ts-jest continues to
handle
.ts/.js.predictionsJest config no longer pins thenodeexport condition.It provides a global
moduleNameMapperstub for@aws-amplify/storage(predictions only consumes
getUrl), which removes the need for aresolution pin. Existing per-test
jest.mock(...)overrides still work.yarn testpasses for thestorage,aws-amplify, andpredictionspackages. Package-level coverage thresholds respected.
Checklist
yarn testpassesChecklist for repo maintainers
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.