fix: trimCanvas import issue in getTrimmedCanvas method #138
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
* ci: migrate to GitHub Actions w/ matrix - Travis CI is only _pseudo_-free after the .org -> .com merge - c.f. https://travis-ci.community/t/org-com-migration-unexpectedly-comes-with-a-plan-change-for-oss-what-exactly-is-the-new-deal/10567 - and honestly, has glitches occasionally where my builds don't run or I can't manually re-run either - GH Actions are _actually_ free for OSS / public repos - and also offer a decent level of composability that's resulted in a huge surge in reusable community Actions (vs. CircleCI orbs etc haven't had nearly as much adoption) - and I've had a mostly great experience on them so far as well - also add a matrix for all Node LTS and all OSes - and upgrade to the Codecov GH Action, since the bash uploader was deprecated (https://about.codecov.io/blog/introducing-codecovs-new-uploader/) - this is basically a mix of the Actions I helped set-up in rpt2's repo and the TSDX templates: - https://github.com/ezolenko/rollup-plugin-typescript2/blob/03cfb048adcf39de56bd4566dc9a7300534a3cc1/.github/workflows/nodejs.yml - https://github.com/jaredpalmer/tsdx/blob/2d7981b00b2bf7363a3eeff44ff5ff698ba58c8c/templates/basic/.github/workflows/main.yml - plus upgrades to all the Actions, for instance, newer `setup-node` now has caching built-in * fix(ci): remove Node 18 due to node-canvas installation issues - per the in-line comment, there are some upstream issues in node-canvas that is causing installation to fail on Node 18 - made a TODO to add Node 18 back to the matrix once it's properly supported upstream
- use the "CI" workflow on the `main` branch only - note that it _is_ case-sensitive, so had to put the workflow name in caps for the badge to work properly - basically same as how the previous badge only reported on `main` - missed this one tiny detail during the migration - did a search this time to make sure I replaced Travis everywhere
- instead of the custom JS code I wrote
- I was thinking of separating that into a plugin, but someone already
did that!
- and, importantly, also covered submodules, unlike _most_ of the
externals plugins
- see regex here: https://github.com/Septh/rollup-plugin-node-externals/blob/11a7b4454f57c76436e71ecead0cc59ab0cc3b80/src/index.ts#L106
- put it _before_ `node-resolve` as the docs state
- reduces my code a bit, which is always nice, and will make it easier
to split my Rollup config into its own "preset" package later on
…ssue - Per b14060c, I ran into a bug in rpt2 when using it as a `configPlugin` and enabling declarationMaps - that bug was actually due to some code _I_ wrote in rpt2 and an edge case I didn't know existed (`configPlugin` usage means Rollup has no `output`) - so I eventually fixed it myself too - and in the past month I stepped up to help maintain rpt2 and fixed a ton of issues and improved lots of things per the release notes :) - since declarationMaps now work while using as a `configPlugin`, remove the workaround `tsconfig.rollup.json`
- this is my own tsconfig base built on top of @tsconfig/strictest
- as you can see, it just reduces all the non-type-checking related config that @tsconfig/strictest doesn't cover
- TS doesn't yet support package `exports` for tsconfigs yet, hence the longer path into the package
- Relative paths in tsconfig bases are also relative _within_ node_modules, so have to repeat any relative paths here
- And due to this, I think it's better right now for `build` to extend the base here instead of the `build` tsconfig in `@agilgur5/tsconfig`
- I didn't fully think that one through when I made it tbh, though it still serves as a good example tsconfig
- Maybe that will be different in the future if TS changes or otherwise comes up with a solution to this
* deps: migrate from Enzyme to RTL
- Enzyme only has unofficial support for React 17 and no support what-so-ever for React 18, so forced to migrate
- RTL is most certainly more of an integration testing library than a unit test library
- can't access the instance through RTL, even though this component actually _needs_ to use refs (bc canvas)
- I managed to hack around this by using refs, but I honestly wasn't even sure that this was possible
- notably hooks like `useRef` don't work since they can only be used inside of a React component or another hook
- but callback refs and `createRef` do work fortunately
- tried to use `react-dom-instance` initially, as suggested in blog post by the author, but it only works with React 16
- in React 17 and React 18 it just returns `false` and doesn't get an instance
- `querySelector` is an escape hatch that is not recommended, enough so that I read through the docs multiple times before finding it was even possible
- was using `document.querySelector` directly before finding this
- somewhat replaces Enzyme's `exists`
- `rerender` is also a bit of an escape hatch
- notably, you can't set props directly on a `ref`, `.props` is a read-only property, so had to find some alternative to be able to run the tests
- replaces Enzyme's `setProps`
- had to change `clearOnResize` to an optional type as it was required without this
- despite that it's in `defaultProps`; the interface itself can't infer this, so probably makes sense to make it optional here as well
* fix lint issues
- seems to work without any component modifications needed on React 18 - React 18 was mostly _deprecations_ and new features per the upgrade guide: https://reactjs.org/blog/2022/03/08/react-18-upgrade-guide.html - React 18 has concurrent mode opt-in, which is technically breaking, but doesn't seem to effect this library (fortunately) - ensure tests pass with React 18 - update devDeps to React 18, react-dom 18, @types/react 18 - update devDeps to RTL 13 which supports React 18 - see previous commit for the migration from Enzyme to RTL since Enzyme never had official support for React 17, let alone React 18 - update example to use `createRoot` instead of `ReactDOM.render` - this is one of the opt-ins to React 18 mode, and doesn't seem to change anything about the example (just no warnings anymore)
- update/fix "build status" badge according to Shields directions - remove old `nodei.co` badge - doesn't always load and isn't very useful anymore - removing it from all of my NPM projects - also change Installation to a codeblock instead of just backticks - easier to read as a block and easier to copy (full horizontal length) - and more consistent too
* update to use LTS Node 18 - Node 14 [fails to install](https://github.com/agilgur5/react-signature-canvas/actions/runs/12562533381/job/35023139557?pr=112) - ```txt Run actions/setup-node@v3 Attempting to download 14.x... Not found in manifest. Falling back to download directly from Node Error: Unable to find Node version '14.x' for platform darwin and architecture arm64. ``` - keep Node 16 for now, although it is out-of-support - ensure continuous range testing; should bump that later too * try 16.x on Ubuntu only for now - that one [passed](https://github.com/agilgur5/react-signature-canvas/actions/runs/12562533381/job/35023139677) - 18.x and other 16.x versions failed to install `node-canvas`, c.f. PR comment
* fix(ci): update `codecov-action` for tokenless uploads - per https://docs.codecov.com/docs/codecov-tokens#uploading-without-a-token, it seems a newer version is required now * token is apparently needed for `main` and other "unprefixed" branches - c.f. https://docs.codecov.com/docs/codecov-tokens#tokenless-on-unprotected-branches * remove token and change Codecov settings apparently according to https://docs.codecov.com/docs/codecov-tokens#enabling-tokenless-uploads-for-public-repositories, there is a setting to enable this, however the setting itself does not say "public repos only" confusingly....
fix(ci): run GH action on PRs as well - `push` works for all PRs from my own branches, but not for PRs from forks from external contributors - see also https://github.com/agilgur5/mst-persist/blob/4f8b9f116d1645112ac2eb790d41f007916b9ff0/.github/workflows/ci.yml#L2
…5#115) deps: update `node-canvas`, Node, + `npm audit fix` - update `node-canvas` to latest v3 to work with newer, non-EoL Node versions - also less deps like `node-pre-gyp` etc - update CI matrix to use LTS Node 18+ - update `actions/checkout` and `actions/setup-node` to use non-EoL Node runtimes as well - the primary breaking change in updating [both to their](https://github.com/actions/checkout/releases/tag/v4.0.0) [respective v4 releases](https://github.com/actions/setup-node/releases/tag/v4.0.0) is the Node runtime update - run `npm audit fix` after
) * deps: update peerDeps to support React 19 without warnings - react 19 is also incompatible with `@testing-library/react` 13, so it needs updating as well, see next commits * typings fixes required in new version * update @testing-library/react to compatible 16.1 version - react 19 support in https://github.com/testing-library/react-testing-library/releases/tag/v16.1.0
* deps: update `concurrently` to latest 9.1.2 * deps: update `jest` et al to latest v29.7.0 - this transitively updated a bunch of Babel et al deps - set `overrides` for the `canvas` version to ensure v3 usage - newer `jsdom` should support this normally later: jsdom/jsdom@8955c99
- use `createRoot` for `react-dom` rendering - use functional components with hooks syntax - add link to `ref` docs - consistently use `sigCanvas` as name - don't switch b/t `sigPad` and `sigCanvas` - consistently format `example/` dir with `ts-standard` - was previously ignored for less diff, but since the example is re-written with newer syntax, good timing to reformat it too
- using `parcel` same as the `example/` dir and `gh-pages` branch - as CRA is now deprecated - also modify some whitespace in this section for better readability of the raw markdown
- I missed this link in f3108fd - (forgot another link existed until I re-read the top of the README)
- `react` and `prop-types` are peerDeps, so their `@types/` packages should be too - and since they're peers now, we can [mark them as optional](https://docs.npmjs.com/cli/v11/configuring-npm/package-json#peerdependenciesmeta) as well (since they're only needed for TS usage) - this should also allow for users to be able to install a range of `@types/react` as well, instead of just v19
- all the things I've added since ~v1.0.0-ish - this branch has been laying here for a few years now too and I think I forgot about it 😅
- the API surface is entirely backward-compatible, hence why this is releasing as SemVer minor - first official release of the TypeScript rewrite - this has been on the `main` branch for a few years (c.f. ecb0374 et al), but had not been released until now - as it is natively in TS now, `@types/react-signature-canvas` is no longer necessary - please file an issue if there are any problems with the typings! this is released as an `alpha` just in case! - as it is natively in TS now, the build process needed quite a lot of updating - it's now built with Rollup (as this is a library, not an app) and various plugins (some of which I help maintain, like `rollup-plugin-typescript2`) instead of Webpack v1 from like a ~decade ago (did Rollup even exist back then?) - there is now a native ESM build and a UMD build for backward-compatibility - _note_: the UMD build may be removed at a later date during a SemVer major update/breaking change - please file an issue if there are any problems with the build / distribution! this is released as an `alpha` just in case!
- I used [my fork of `changelog-maker`](https://github.com/agilgur5/changelog-maker) for some time, but now my process is more involved - I manually categorize, summarize, and minimize changes - a good chunk of this could be automated by reading the semantic / conventional commit titles, but not entirely (e.g. when a change can fit multiple types or scopes) - I use GitHub Releases' "Generate release notes" feature for the first draft / list of commits + PRs - unlike `changelog-maker`, GH's feature does not require a `PR-URL: <url>` to exist in the commits to be able to reference the PR - I'd like to use a more cross-platform tool that is not reliant on GH, but this is the easiest option right now - removing this outdated devDep also removes any remaining vulns in the devDeps - these didn't affect prod built code and were not exploitable, so were super low priority to fix mainly for less scanner noise Future Work: 1. better automate the changelog 1. make a GH job to publish to NPM and create the GH release - start with manual version bumps for now, but with releases via CI
In TS 4.7+, when using newer `moduleResolution` options such as `node16`, `bundler`, and `nodenext`, the types _must_ be defined in `package.json#exports` to be properly detected
- since `typings` is now considered a legacy alias and can't be used inside of `exports`, update all references to consistently use `types` - `package.json#typings` -> `types` - `typings` folder -> `types` - for similar reasons, use consistent relative path `./` in `package.json` - so `types` matches `exports.types`, `main` matches `exports.require`, and `module` matches `exports.import` _exactly_
- otherwise it isn't detected and types only happen to work [due to a TS bug](https://github.com/arethetypeswrong/arethetypeswrong.github.io/blob/6ab2f0a7bf5ab4a64f57fe71df7f58506f227362/docs/problems/FallbackCondition.md)
…#132) - a `napi-build-utils` bug caused a bug in `prebuild-install` which causes `canvas` to fail to fetch prebuilds (and CI to partially fail) during installation of `canvas` on Node 22.14+ - update the patch in the lockfile - while at it, bump `canvas` too to a newer version with less deps
- newer versions of TS 4.7+ with `moduleResolution` set to `node16` and later require that, if `package.json#exports` is defined, then `package.json#exports.types` must also be defined. - this version should now work with `moduleResolution` set to `node16`, `bundler`, or `nodenext`
- following the [`tj-actions` supply chain attack](https://www.stepsecurity.io/blog/harden-runner-detection-tj-actions-changed-files-action-is-compromised), figured I should harden some of my small repos too - follow [OpenSSF Scorecard best practices](https://github.com/ossf/scorecard/blob/43d5832d25ccc597a9b94926b6ad43da25204085/docs/checks.md) - specifically "Pinned Dependencies" and "Token Permissions" - In the future, may add [`falco-actions`](https://github.com/falcosecurity/falco-actions) etc for anomaly detection - see also https://sysdig.com/blog/detecting-and-mitigating-the-tj-actions-changed-files-supply-chain-attack-cve-2025-30066/ - based off OSS Falco, more powerful than and without restrictions unlike [`harden-runner`](https://github.com/step-security/harden-runner), although it doesn't have proactive egress blocking via an allowlist as `harden-runner` does 😕 - right now, adding those actions could arguably add _more_ surface area given the small usage of the current actions (could be a premature optimization rn) Co-authored-by: StepSecurity Bot <[email protected]>
* gh: create a bug report issue template + template chooser config
- have gotten a decent number of issues with no reproductions, so finally decided to make that a requirement for bug reports
- require a reproduction, environment details from `envinfo`, and an attestation that the issues were searched
- also have gotten a number of duplicates, some within an hour of each other no less
- in the issue template and in the template chooser, also refer folks to SO for support
- since more beginners find themselves using this library, make them more aware that issues are for bug reports and not support requests, which are better suited for SO or consulting / contracts (than for volunteer maintainers to help debug every use-case for free).
- add links to SO, SO's MCVE/MRE docs, upstream `signature_pad`, and GitHub's communication docs
* also search existing issues
and give directions on what to do in case of a closed duplicate issue
* backslashes apparently not necessary for single new line
- Fixed runtime error "TypeError: (0 , import_trim_canvas.default) is not a function" caused by incompatible default/named exports in trim-canvas. - Updated import to a universal approach compatible with: Node.js, TypeScript, and all package managers/bundlers. - getTrimmedCanvas now safely trims canvas regardless of environment.
…to support CJS and ESM builds
Hamza65523
force-pushed
the
fix/signature-clear-bug
branch
from
a40e872 to
3ece5cb
Compare
|
This appears to have been superseded by #139. |
agilgur5
added
solution: duplicate
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Fixed a TypeError caused by incorrect default import from 'trim-canvas'.
✅ Changes:
import trimCanvas from "trim-canvas"withimport * as trimCanvas from "trim-canvas"and handled export properly for production build.
This resolves
TypeError: trimCanvas is not a functionduring build/runtime.