Repository navigation
Conversation
Formatters and plugins installed via npm often depend on other packages through version ranges, so a new release of a transitive dependency can silently change formatting output (e.g. prettier-plugin-sql -> sql-formatter@^15). Allow pinning such dependencies via npm's `overrides` field in the generated package.json, for prettier, ESLint and tsfmt: - lib: new create(...) overloads taking `dependencyOverrides`; existing signatures (incl. the public TsFmtFormatterStep.State constructor) are kept. Entries are sorted and JSON-escaped. When no overrides are set, the generated package.json is byte-identical to before, so existing node_modules directories and cached step state stay valid. - lib: replace placeholders literally, so values starting with `$` (npm override references such as "$prettier") don't break rendering. - plugin-gradle: `dependencyOverrides(Map)` on prettier/eslint/tsfmt. - plugin-maven: `<dependencyOverrides>` on prettier/eslint/tsfmt. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MTr23xWrpXPm9jbQGavZ8E
This branch has not been deployed
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.
Closes #2534
Why
npm-based formatters and their plugins often depend on other packages through version ranges. A new release of such a transitive dependency can silently change the formatting output, even though none of the versions configured in Spotless changed. The issue's example:
prettier-plugin-sqldepends onsql-formatter@^15.x; a newsql-formatterrelease changed the output and broke builds with checked-in SQL files.Today there is no way to pin those transitive dependencies, because Spotless generates the
package.jsonitself.What
As suggested in the issue discussion, a
dependencyOverridesoption for all npm-based formatters (prettier, ESLint, tsfmt), configured just likedevDependencies. The entries are written to npm'soverridesfield of the generatedpackage.json.Gradle
Maven
Details
PrettierFormatterStep,EslintFormatterStepandTsFmtFormatterStepget acreate(...)overload with adependencyOverridesmap. The existing signatures (including the publicTsFmtFormatterStep.Stateconstructor) are kept and delegate with an empty map, so there are no breaking API changes.package.jsonis byte-identical to before. Existingnode_modulesdirectories (whose name is a hash of thepackage.json) and cached step state stay valid, so there is no forced reinstall. A characterization test asserts this.package.jsonand therefore of the step state. Changing them makes the step different and triggers a freshnpm installinto a new directory.package.jsontemplate were replaced withString.replaceAll, which treats$in the replacement as a group reference. Values like npm's override references ("$prettier") would have thrown. The replacement is now literal.Scope / limitations
name -> versionform of npm overrides is supported (including$references), not nested override objects. That covers the use case of the issue; nested overrides could be added later if needed.<devDependencies>,<dependencyOverrides>can't express scoped package names (@scope/pkg), because@is not valid in an XML element name. AdependencyOverridePropertiescounterpart todevDependencyPropertiescould be added if you'd like. I kept it out to keep this PR focused.Tests
Written test-first. Every new test was run and seen failing for the right reason before the implementation:
package.jsonbyte-identical to beforeNpmPackageJsonRenderingTest(lib)overridesblock;$references kept; JSON escaping; present in all three templatesNpmPackageJsonRenderingTest(lib)npm install: prettier +prettier-plugin-sql,sql-formatterpinned to15.7.0→15.7.0installed and formatting works (without the fix npm installs15.9.0)PrettierFormatterStepTest.DependencyOverridesPrettierFormatterStepTest.equality,EslintFormatterStepTest,TsFmtFormatterStepTestPrettierIntegrationTest.dependencyOverridesPinTransitiveDependenciesPrettierFormatStepTest.dependencyOverrides_pin_transitive_dependenciesRan locally:
:lib:test: 97/97:testlib:testNpm: 45/45:plugin-gradle:testNpm: 57/58 passed, 1 skipped:plugin-maven:testNpm: 37/38 passed, 3 skippedspotlessCheckfor lib, testlib, plugin-gradle and plugin-maven: cleanThe two failures look unrelated to this PR:
TypescriptExtensionTest.useEslintStandardWithTypescriptRulesandTypescriptFormatStepTest.eslintStyleguideStandardWithTypescriptboth fail withFailed to load parser '@typescript-eslint/parser' … (reading 'BarBarToken'). I re-ran the Gradle one on unmodifiedmainand it fails the same way there. The cause is a newer TypeScript resolved through a version range, the very drift this PR makes pinnable.Side note, possibly worth a separate issue: locally,
./gradlew testNpmreportsNO-SOURCEfor me on Gradle 9. The customtest<Tag>tasks registered inspotless.special-tests.gradle.ktsdon't settestClassesDirs/classpath, so they may not run any tests (CI runs./gradlew testNpm). I wired them up with a local init script to run the suites above. Happy to open a separate issue or PR if you can confirm.