refactor: move the esm.run provider into Importmap::EsmRun - #42
Merged
Merged
Conversation
packager.rb was at 811 lines against the 800-line ceiling, so the next addition to the command path had nowhere to go but another round of comment-trimming. The largest block of fork-only behaviour in that upstream-owned file was the esm.run path — nine constants and methods that between them know one CDN's URL shapes — and moving it out shrinks the diff the next `git merge upstream/main` has to reconcile. 811 → 743. The one request esm.run makes still goes out through Packager#fetch_remote. HttpRetries#with_retries raises self.class::HTTPError, so a collaborator with its own error classes would raise something ProviderChain's `rescue Importmap::Packager::Error` doesn't catch, and a jsDelivr miss would crash the CDN fallback chain instead of moving on to the next provider. fetch_remote and get_response take the retry's description as a keyword so that message stays exactly what it was. No behaviour changes and no documented setting moved: Importmap::Packager.esm_run_resolver, which docs/configuration.rb lists as public config, delegates to Importmap::EsmRun.resolver the way retry_attempts already delegates to HttpRetries. ## Test coverage - test/esm_run_test.rb: the collaborator's own surface — provider and URL recognition, url_for, the import rewrite and its two-version warning, the Packager.esm_run_resolver delegation, a resolved version and a 404 - test/packager_test.rb, test/commands_test.rb: unchanged, and the regression net for the move — the esm.run cases and the live `--from esm.run` pins Closes #31 Claude-Session: https://claude.ai/code/session_01B8NAp4J4RS7BBUDDQtRA7W
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.
Summary
lib/importmap/packager.rbwas at 811 lines against the 800-line ceiling in.claude/rules/coding-style.md, so the next addition to the command path hadnowhere to go. The esm.run path — the largest block of fork-only behaviour left
in that upstream-owned file — moves to
lib/importmap/esm_run.rb: 811 → 743,and the diff the next
git merge upstream/mainhas to reconcile gets smaller,which is what
.claude/rules/upstream-sync.mdasks of every change there.Moved, as
Importmap::EsmRun:Packager::ESM_RUN_PROVIDEREsmRun::PROVIDERPackager::ESM_RUN_CDNEsmRun::CDNPackager::ESM_RUN_URL_REGEXPEsmRun::URL_REGEXPPackager::ESM_RUN_IMPORT_REGEXPEsmRun::IMPORT_REGEXPPackager.esm_run_resolverEsmRun.resolver— still readable and writable under the old namePackager#esm_run?EsmRun.provider?— the Packager method stays, as a delegatorPackager#rewrite_esm_run_importsEsmRun.rewrite_importsPackager#import_from_esm_runEsmRun#importsPackager#resolve_esm_run_versionEsmRun#resolve_versionEsmRun.url_for(name, version, subpath), which the two URL-building sites sharedNo behaviour change, and nothing user-facing:
bin/importmap pin --from esm.runprints the same sentences and writes the same pins. The
:nodoc:ESM_RUN_*constants are the only removed names, and
CHANGELOG.mdgets a### Changedbullet saying where they went.
lib/importmap/esm_run.rb— new, 118 lineslib/importmap/packager.rb—+17 −85lib/importmap/module_inspector.rb,lib/importmap/package_graph.rb— twocomments repointed at the regex's new home
CLAUDE.md—esm_run.rbadded to the architecture map and the fork-only listdocs/change:docs/app/views/docs/pages/configuration.rbdocumentsImportmap::Packager.esm_run_resolver, which still answersCloses #31
Test plan
bundle exec rake test— 448 runs, 1714 assertions, 0 failures, 0 errors, 0 skips, with bun installed, so the--minifypaths ran and the live jspm / jsDelivr / npm-registry cases went outtest/esm_run_test.rbwritten first and seen red (cannot load such file -- importmap/esm_run)test/commands_test.rb's live--from esm.runcases, unchanged and green —pin "md5" # @2.2.0 (esm.run)plus the four dependency pins the bundle needsDeviations & judgment calls
Deviations
(−68).
packager.rbhad grown from 799 to 811 since the issue was filed,and the two delegators plus the
description:keyword add 17 lines back.Discoveries
docs/app/views/docs/pages/configuration.rbdocumentsImportmap::Packager.esm_run_resolveras public config, so it could not justmove. It is now a delegator to
Importmap::EsmRun.resolver— the same shaperetry_attempts/retry_waitalready use forImportmap::HttpRetries.packager.rbever referenced theESM_RUN_*constants oresm_run?.commands.rbnames esm.run only in comments and inpin_esm_run_dependencies, which works off the dependency listdownloadreturns. Two comments in
module_inspector.rbandpackage_graph.rbpointedat
Packager::ESM_RUN_IMPORT_REGEXPand were repointed.Judgment calls
Packager#fetch_remoteinstead of keepingits own
Net::HTTP.get_response+with_retries+handle_failure_response.HttpRetries#with_retriesraisesself.class::HTTPError, so anEsmRunwithits own error classes would raise something
ProviderChain'srescue Importmap::Packager::Errordoes not catch — a jsDelivr miss wouldcrash the CDN fallback chain instead of reporting and trying the next
provider. Routing the one request through the Packager keeps every error in
the Packager hierarchy with no translation layer. Two costs, both accepted: an
additive
description:keyword onfetch_remote/get_responseso theretry's message stays exactly
resolving <uri>, and an unexpectednon-retryable error on that request is now wrapped in
HTTPErrorrather thanraised raw.
esm_run.rbdoes notrequire "importmap/packager". It namesImportmap::Packager::PACKAGE_SPEC_REGEXP,::Errorand::HTTPErrorinsidemethod bodies only, which is exactly what
package_graph.rbdoes withPackager::Unvendorable. No require cycle, andtest/esm_run_test.rbrequires both, as
package_graph_test.rbdoes.Packager#esm_run?kept as a one-line delegator rather than deleted. It ispublic fork API with no
:nodoc:; removing it is not part of this refactor.CHANGELOG.mdentry under a new### Changedeven though no behaviourchanged, because the
:nodoc:ESM_RUN_*constants did move and someone mayhave reached for them.
https://claude.ai/code/session_01B8NAp4J4RS7BBUDDQtRA7W
Summary by cubic
Moves the esm.run provider out of
Importmap::Packagerinto a newImportmap::EsmRunclass, gettingpackager.rbback under the 800-line ceiling and shrinking the next upstream merge diff. Pinning with--from esm.runand reading or writingImportmap::Packager.esm_run_resolverbehave exactly as before.:nodoc:Packager::ESM_RUN_*constants are gone; they now live asImportmap::EsmRun::PROVIDER,::CDN,::URL_REGEXP, and::IMPORT_REGEXP.Packager#fetch_remote, so failures still raiseImportmap::Packager::Errorand the CDN fallback chain keeps working.Packager#esm_run?stays as a one-line delegator so existing callers don't break.test/esm_run_test.rbcovering provider and URL detection, import rewriting, version resolution, and the resolver delegator.Closes #31
Written for commit 730dd3b. Summary will update on new commits.