Conversation
Both filters were pinned to master, so work staged on a long-lived version branch ran no CI at all. The 'v*' pattern covers the current v18 branch and any future major without needing another edit. The release workflow is unaffected - it stays guarded by `if: github.ref == 'refs/heads/master'` and only runs on its cron. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ReactSVG fetches and injects in lifecycle code, so it is client-only. Publishing the directive means Next.js App Router users no longer need to hand-wrap the component in their own "use client" boundary (#2888). Rollup strips file-level directives while bundling, so the directive is added via output.banner on the CJS and ES configs. Terser classifies "use client" as a non-standard directive and drops it by default, so compress.directives is disabled to keep it in the minified CJS bundle. The dist/index.js shim carries the directive too, since it is the CJS entry point. UMD bundles are script-tag targets with no server components boundary to mark, so they are left alone. The ssr example is now a server component importing ReactSVG directly. Its loading prop was removed because function props cannot cross the server/client boundary. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Nothing guarded the published package shape. publint audits package.json correctness, attw verifies TypeScript consumers resolve types across node10/node16/bundler resolution. Both run as a postbuild hook because they inspect dist/, which does not exist when the check:* glob runs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
React removed its own UMD builds in v19, so script-tag usage of react-svg already required pinning React 18 or earlier. No modern peer library in the ecosystem ships UMD. - rollup.config.mjs: remove the UMD_DEV/UMD_PROD configs. With only CJS and ES left, every bundle externalises the same set (peer dependencies plus dependencies), so getExternal collapses to a single `external` predicate. - Delete the UMD jest configs and their test:umd / test:umdprod scripts. - Delete examples/umd-dev and examples/umd-prod, and their README entries. - Remove the README unpkg/UMD installation subsection. Script-tag consumers can pin react-svg@^17. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
React 19 ignores propTypes entirely, so the runtime checks were dead weight for anyone on a current React while still shipping prop-types and @types/prop-types as runtime dependencies. TypeScript types are the actual prop contract. Removing them also removes the only difference between the dev and prod CJS bundles beyond minification, which bundlers apply themselves, so the two collapse into a single unminified dist/react-svg.cjs.js and the dist/index.js NODE_ENV shim goes with them. With every dependency externalised, the node-resolve, commonjs, replace and terser Rollup plugins no longer affect the output (verified byte-identical) and are dropped.
Replaces the tsc -> compiled/ -> Rollup + Babel pipeline with a single tsdown build. Node ESM consumers were resolving to the CommonJS build because Node ignores pkg.module, so the package now ships an exports map with matching .d.mts/.d.cts pairs. main/module/types stay for webpack 4 and TypeScript node10 resolution. Output still targets es2019 so webpack 4 can parse it. @babel/runtime is gone, leaving @tanem/svg-injector as the only runtime dependency. The ESM bundle is now a .mjs file, which TypeScript always emits as ESM, so test:es runs Jest in ESM mode. src is published so the declaration maps resolve. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Class internals forgo React Compiler optimisation, and every supported peer now ships hooks. The lifecycle methods map onto one effect: the dependency list replaces `shallowDiffers`, and a per-run `isActive` flag replaces `_isMounted`, which also covers a dependency change landing while the previous injection is still in flight. `defaultProps` is gone, since React 19 ignores it on function components. The peer range becomes `^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0`. React supports a major as a unit and only backports to the head of a line, so pre-16.8 minors were never individually supported. The test matrix swaps 16.0 and 16.3 for 16.8. Two behaviour changes fall out of this and are documented in MIGRATION.md: `ref` now resolves to the wrapper DOM element rather than the class instance, and re-injection is limited to props that affect the injected SVG, so inline callbacks and wrapper-only props no longer force a re-fetch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Arrows are the convention elsewhere in the codebase. An anonymous arrow gets no inferred name, so `displayName` is set explicitly to keep React DevTools showing "ReactSVG" rather than "ForwardRef". Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Nothing prevented a size regression from landing, despite the README advertising a minzip badge. `npm run size` checks the gzipped size of both published bundles against budgets declared in package.json, and runs after `build` in `npm test` so CI and local runs agree. Budgets are gzip rather than size-limit's default brotli so the gated number matches the metric the README badge reports. Current sizes are 1.66 kB (ESM) and 2.11 kB (CJS); the budgets sit at 2 kB and 2.5 kB, roughly 20% headroom. Only @size-limit/file is installed, so the check measures our own output and does not shift when @tanem/svg-injector is bumped. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The README never mentioned build-time SVG tooling, which is the right answer for the common case of SVGs that live in the repo. State the scope explicitly: injection earns its network request and wrapper elements when the URL is only known at runtime and the markup has to be reachable by CSS. The existing security note only covered inline SVG strings. Fetched sources carry the same risk, so cover them in their own section: the `evalScripts: 'never'` default, sanitising with DOMPurify in `beforeInjection` for the vectors `evalScripts` doesn't touch (event handler attributes, `javascript:` hrefs), validating the `src` URL, and the page-wide reach of a `<style>` element inside an injected SVG (#2077), which sanitising does not address. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot code review was the last surface tied to .github/copilot-instructions.md; it went GA with root AGENTS.md support in June 2026, so the vendor-specific path has no remaining reader and is deleted rather than kept as a pointer. CLAUDE.md imports AGENTS.md because Claude Code does not read AGENTS.md natively, and an import beats a symlink: symlinks need Developer Mode or admin rights on Windows checkouts. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The single-version verification snippet chained the install and the Jest run in one block after a cd, but only the install belongs in the version directory: the Jest config resolves rootDir from process.cwd(), so running it from test/react/<version> exits before it finds a config. scripts/test-react.ts always ran Jest from the root; the instructions had drifted from it. The release section is new. Nothing in the repo records that the version bump comes from PR labels, that an unlabelled or multi-labelled PR blocks the release outright, or that CHANGELOG.md, AUTHORS and both version fields are generated - all of which an agent can only get wrong. Line wrapping is now consistent at 80 columns; prettier ignores *.md, so nothing enforced it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The file was restating comments that already sit next to the code they constrain: src/ReactSVG.tsx documents the forwardRef requirement, the callbacks ref, the narrow effect dep list and the teardown guard at each site, and browser.spec.tsx explains the @jest/globals import in the same words. Those duplicates cost context in every session and can now drift from the code they describe. What remains is the load-bearing invariant an agent could violate without opening the file (don't collapse the two wrappers), plus the rules with no home in the code at all: the release labels, the matrix policy, the CodeSandbox template pinning. Also dropped: the setupJest act suppression (visible in an 18-line file), the import-sort convention (ESLint enforces it), and the step-by-step matrix procedure, which described a once-a-year task in every context window and pointed at scripts/test-react.ts anyway. 978 words to 530. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Reverts part of f83582c. `engines` isn't advisory everywhere: yarn classic hard-fails installs on a mismatch, and yarn 1 with a lagging Node is common in exactly the legacy codebases v18 went to lengths to keep working (top-level main/module/types, ES2019 output). Nothing in the package needs Node 22. The output targets ES2019, there's no Node-specific runtime behaviour, and React ships no engines field either. So the field could fail installs for the target audience while protecting nobody. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
express.static has no extension to infer a type from and falls back to application/octet-stream, which svg-injector rejects for any URL that doesn't end in .svg. Nothing rendered and nothing errored, so the example had been silently blank. The README already claimed the server responded with an appropriate content type. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both examples request a deliberately missing SVG to show the fallback and loading props. Vite's default SPA fallback answered that with index.html and a 200, so svg-injector neither injected nor errored and dropped the callback: fallbacks rendered nothing and loading spun forever. appType 'mpa' turns the miss back into a 404. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A symbol carries a viewBox but no width or height, so each extracted icon stretched to the width of the page. Sized through beforeInjection, as the data-url example already does. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Every renovate PR was labelled `internal` by the `:label(internal)` preset, including bumps of `@tanem/svg-injector`, the only entry in `dependencies` and the one thing that reaches consumers. #3586 shipped a new injector version to users and was filed under ":house: Internal" as a patch. Runtime dependency bumps now get `bug`, and majors get `breaking` with automerge off, since an injector major has forced a react-svg major before: v11 dropped IE support and produced v17. `semanticCommits` was auto-detected as enabled, so renovate emitted `chore(deps):` and `fix(deps):` prefixes. Nothing reads them - the release type comes from the PR label - and the generated changelog quotes PR titles verbatim, so the prefixes only add noise. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
None of these were written down, so the v18 branch drifted into conventional-commit prefixes that nothing in the repo reads. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A personal writing preference rather than a constraint of this project, so it belongs in a global agent config. This file is for rules the code and config don't already state about react-svg itself. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Code reviewFound 1 issue:
react-svg/test/bundles.spec.ts Lines 10 to 18 in 24fe4c0 react-svg/config/jest/config.src.js Lines 30 to 32 in 24fe4c0 🤖 Generated with Claude Code - If this code review was useful, please react with 👍. Otherwise, react with 👎. |
`test/bundles.spec.ts` reads `dist/` but was matched by
`config.src.js`'s glob, so `npm run test:src` failed with ENOENT until a
build had been run. `test/node.spec.ts` reads `dist/` too, via
`require('..')`, and passed vacuously without one: a "Cannot find
module" error still satisfies the old
`not.toThrow('ReferenceError: window is not defined')` assertion, so the
SSR check was silently disabled.
Both now run under `test:dist`, which the full gate reaches after
`build`. `config.src.js` keeps its glob so a new spec still runs by
default.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A class declaration doubles as a type describing its instances, so `Omit<ReactSVG, 'src'>` was valid before the function component rewrite and now fails with TS2749. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Fixed in 9860947, addressing the review finding above.
|
Props become a summary table of name, type and default plus a `####`
heading per prop, so each one has an anchor an issue reply can link to.
The ref and re-injection paragraphs become labelled subsections for the
same reason. The error-routing behaviour is stated once above the props
instead of repeated in four bullets.
The kitchen-sink example no longer sets `evalScripts="always"` and
`httpRequestWithCredentials={true}`, which contradicted the Security
section two sections below it.
Modernises the package for v18. Every ticket landed as its own commit on this branch rather than as interim PRs:
.github/workflows/release.ymlruns on a Monday cron againstmasterwith no gate, so a half-landed major would publish itself.Full consumer-facing detail is in
MIGRATION.md. Summary:Breaking
^16.8.0 || ^17.0.0 || ^18.0.0 || ^19.0.0), andReactSVGis now a function component built on hooks.refresolves to the wrapper DOM element instead of the class instance, re-injection is limited to props that affect the injected SVG, and theStatetype export is gone.exportsmap added. Paths intodistare no longer reachable. Top-levelmain/module/typesstay for webpack 4 and TypeScriptnode10.dist/react-svg.cjs/.mjs, with.d.cts/.d.mts); output still targets ES2019 and@babel/runtimeis no longer a runtime dependency.propTypesdropped. TypeScript types are the supported prop contract.Non-breaking
"use client"in the published bundles.sideEffects: false.publint,arethetypeswrongand asize-limitbudget.AGENTS.md;.github/copilot-instructions.mddeleted now that Copilot code review readsAGENTS.md.enginesfield: yarn classic hard-fails installs on a mismatch, and the package has no actual Node floor.bugrather thaninternal, so they stop being filed under Internal in the changelog, and majors are held for review asbreaking.Verification
npm testgreen, including the React matrix (16.8, 16.14, 17.0, 18.0, 18.3, 19.0, 19.1).npm packtarball of this branch and render their SVG;examples/ssradditionally builds, serves and hydrates undernext build && next startwith a clean console. The v18 packaging needed no fixes. Three pre-existing example bugs surfaced and were fixed along the way.Supersedes #3593, whose
"use client"work landed here asefc30231.