fix(printer,highlighter): declare the dedent dependency - #2231
Open
luantaraschi wants to merge 1 commit into
Open
fix(printer,highlighter): declare the dedent dependency#2231luantaraschi wants to merge 1 commit into
luantaraschi wants to merge 1 commit into
Conversation
Both packages import dedent from their src/, and rolldown leaves it as an external import in the published dist, so a consumer that installs either package on its own gets ERR_MODULE_NOT_FOUND on first run. The root package.json carries dedent as a devDependency, which is why it resolves in the workspace and only breaks for people installing from npm. Verified against the published 0.10.3 tarballs: dist/cli.js of both keeps import dedent from "dedent", while the formatter bundles it inline and needs no declaration. The version matches what the language-server already declares and what the root devDependency pins.
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.
Follow-up to the review I left on #2214, which declares
dedentfor the linter. Same defect, two more published packages. This PR does not touch the linter, so the two can land in either order.Reproduced against the published packages, not the workspace. In an empty project, installing
@herb-tools/printer@0.10.3on its own and running its CLI:@herb-tools/highlighter@0.10.3fails the same way from its owndist/cli.js. Installingdedentclears it. The workspace never sees this because the rootpackage.jsoncarriesdedentas a devDependency, so it resolves for everyone developing in the monorepo and only breaks for people installing from npm.Why these two and not the others. I unpacked the 0.10.3 tarballs and looked for an external
dedentimport in what actually ships:import dedentinsrc/dist/src/cli.tsdist/cli.js:2src/cli.tsdist/cli.js:1src/cli.ts,src/comment-helpers.tsherb-format.jsThe formatter is the interesting row. It imports
dedentinsrc/exactly like the others, but its build bundles everything into a singleherb-format.jswith dedent inlined at line 37, so it has no runtime dependency to declare. I confirmed that by running the published formatter CLI in the same empty project with no dedent installed: it prints its usage instead of failing. So the fix is two packages, not four.Two more I deliberately left alone:
vscodeimportsdedentinsrc/issue-reporter.ts, but the package isprivate: trueand esbuild-bundled, so nothing is published and nothing breaks. A devDependency there would be tidier, not a fix.language-serverdeclaresdedentindependenciesbut its only use is in a test file, so that one is in the wrong section rather than missing. I did not want to remove a dependency from a published package in a PR about adding them, but it is worth a look.No lockfile change needed.
yarn.lockalready resolvesdedent@^1.7.2, which is the exact range the root and the language-server use and the one added here, soyarn install --frozen-lockfilestays satisfied.One thing I could not run. This repo does not check out on Windows: three fixtures under
config/action_view_helpers/have?in their filenames, which NTFS rejects. I built the change through the git object store rather than a working tree, and I verified the two blobs I extracted hash-match the ones onmainbefore editing them, so the diff is exactly the two lines you see. I did not run the test suite. Given the change is two dependency declarations that the lockfile already resolves, CI should be the honest word on it.Unrelated, but you may want to know since I hit it: with
dedentinstalled,@herb-tools/printer@0.10.3's CLI gets past that error and then fails withERR_IMPORT_ATTRIBUTE_MISSINGon a JSON import. Different bug, not touched here.