Repository navigation
feat: add tree-shakeable ESM build (minor) - #218
Open
Roberto286 wants to merge 2 commits into
Open
Roberto286 wants to merge 2 commits into
Roberto286 wants to merge 2 commits into
Conversation
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.
Pull Request Description
Makes the package tree-shakeable by publishing an additional ESM build, and trims the published tarball. No
exportsfield, CJS output unchanged.The generated code is already tree-shake friendly (no module-level side effects,
api/*only import model types). The only missing piece was packaging: until now only a CommonJS ES5 build was shipped, with nomoduleorsideEffects.Changes:
tsconfig.esm.json(new): extendstsconfig.json,module: esnext,outDir: dist/esm,declaration: false.targetstays ES5 (same syntax as the current CJS build), types still come fromdist/*.d.tsviatypings.package.json:module: ./dist/esm/index.js,sideEffects: false, build runstsc -p tsconfig.esm.json,copy:packagealso copiespackage.jsonintodist/esm/(needed forimport { version } from "../package.json"and for bundlers resolvingsideEffects). Newfiles: ["dist"]:docs/(~7 MB) and the.tssources are no longer published next todist/.index.ts: additive root exports ofAxiosManager,Configuration,ConfigurationParameters, so a single API class can be used standalone (new UsersApi(undefined, basePath, axiosManager.axios)) without deep imports into the CJS build.client/client.tsis intentionally untouched: it statically imports all 58 API classes, sonew Fireblocks()still bundles them all. The gain is for consumers who instantiate individual API classes.Type of change
Compatibility
exportsfield: deep imports such as@fireblocks/ts-sdk/dist/network/axiosManagerkeep working.diff -r -x esm -x package.jsonofdist/against master shows onlyindex.js,index.js.map,index.d.ts(the two new re-exports).main.files: ["dist"]: imports of paths outsidedist/(e.g.@fireblocks/ts-sdk/models/...,/docs/...) will no longer resolve. These were never documented entry points, but flagging it.dist/...deep imports (CJS) contains two copies of the classes, soinstanceof FireblocksErroracross copies isfalse. There is no shared or global state.versionfrom JSON; it is only a warning.Porting to the generator templates
All touched files are listed in
.openapi-generator/FILES, so the next generator PR will overwrite them unless the private templates are updated:package.mustache: addmoduleandsideEffectsaftertypings, addfiles, changebuildtotsc && tsc -p tsconfig.esm.json && npm run copy:packageandcopy:packagetocp package.json dist/ && cp package.json dist/esm/.tsconfig.esm.jsonas a supporting file (content as in this PR). It also needs"moduleResolution": "node", otherwiseresolveJsonModulefails with TS5070 undermodule: esnext.index.tstemplate: add the two re-export lines.Note: the stock
supportsES6option oftypescript-axiosdoes (1)+(2) but also raises the CJS build to ES6, changing today's CJS output. It should be replicated in the custom template rather than enabled.How Has This Been Tested?
npm test: 8 suites, 168 tests pass.require('@fireblocks/ts-sdk')exposesFireblocks,TransactionsApi,AxiosManager,Configuration; ESMimport { Fireblocks, BasePath }works under Node; deep import ofdist/network/axiosManagerworks. Installed package contains onlydist/,package.json,README.md,LICENSE.--main-fields=module,main), entry importing onlyUsersApi+AxiosManager:TransactionsApiAxiosParamCreatoroccurrencesdist/models/inputsChecklist: