Package improvements for bundling and tree-shaking#75
Merged
dentarg merged 3 commits intocloudamqp:mainfrom Jul 25, 2023
Merged
Package improvements for bundling and tree-shaking#75dentarg merged 3 commits intocloudamqp:mainfrom
dentarg merged 3 commits intocloudamqp:mainfrom
Conversation
- Add sideEffect: false for better treeshaking - Remove "ava" section - Set subpath exports for per-file importing
dentarg
reviewed
Jul 25, 2023
| @@ -1,4 +1,5 @@ | |||
| { | |||
| "ignorePatterns": ["types/", "dist/", "lib/"], | |||
Member
There was a problem hiding this comment.
I see this is useful when running eslint directly, e.g. npm exec -- eslint . and not npm run lint
Contributor
Author
There was a problem hiding this comment.
Also useful when running an eslint plugin within an editor such as Visual Studio Code.
dentarg
reviewed
Jul 25, 2023
| "rules": { | ||
| "no-console": "off" | ||
| "no-console": "off", | ||
| "@typescript-eslint/consistent-type-imports": "error" |
Member
There was a problem hiding this comment.
I'll drop a link to the docs on this rule as I looked it up: https://typescript-eslint.io/rules/consistent-type-imports/
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.
My application bundler (esbuild) wasn't able to completely tree-shake the
AMQPWebSocketClientimport and was adding anode-modules-polyfills:bufferto the output bundle.This pull request resolves that through these changes:
"sideEffects": false,inpackage.jsontells bundlers to not include an import if nothing is used from that import.index.ts. Eliminates any question if something is going to be removed or not by tree-shaking, since it isn't imported in the first place. End applications will need to use a TypeScriptmoduleResolutionsetting ofnode16,nodenext, orbundlerto use the additional subpath exports.avaconfig."moduleResolution": "node16"to enforce all imports correctly."node"is for node v11 and earlier behavior (more info).@typescript-eslint/consistent-type-importsand fix errors. May also help bundlers if an import is only being included for its types.This also closes #63 because the
AMQPBaseClientwill be importable directly from theamqp-base-client.jsfile:I've tested the changes on my local application.