Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(unstable): vendor cache should support adding files to hashed directories #20070

Merged

Conversation

dsherret
Copy link
Member

@dsherret dsherret commented Aug 5, 2023

This changes the design of the manifest.json file to have a separate "folders" map for mapping hashed directories. This allows, for example, to add files in a folder like http_localhost_8000/#testing_5de71/ and have them be resolved automatically as long as their remaining components are identity-mappable to the file system (not hashed). It also saves space in the manifest.json file by only including the hashed directory instead of each descendant file.

// manifest.json
{
  "folders": {
    "https://localhost/NOT_MAPPABLE/": "localhost/#not_mappable_5cefgh"
  },
  "modules": {
    "https://localhost/folder/file": {
      "headers": {
        "content-type": "application/javascript"
      }
    },
  }
}

// folder structure
localhost
  - folder
    - #file_2defn (note: I've made up the hashes in these examples)
  - #not_mappable_5cefgh
    - mod.ts
    - etc.ts
    - more_files.ts

@dsherret dsherret changed the title feat(unstable): vendor cache should support adding files to hashed directories fix(unstable): vendor cache should support adding files to hashed directories Aug 5, 2023
@@ -932,7 +1043,6 @@ mod test {
json!({
"modules": {
"https://deno.land/x/different_content_type.ts": {
"path": "deno.land/x/#different_content_ty_f15dc.js",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This property was also removed because it's unnecessary. We can derive it from the url and headers.

Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment on lines +84 to +85
} else if let Some(last_index) =
components.iter().rposition(|c| c.starts_with('#'))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume it's not possible to have a URL that has multiple hashes?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There could be multiple hashed components. This selects the deepest hash in order to get the reverse mapping from there.

cli/cache/http_cache/local.rs Outdated Show resolved Hide resolved
@dsherret dsherret merged commit 7b5bc87 into denoland:main Aug 6, 2023
13 checks passed
@dsherret dsherret deleted the feat_unstable_vendor_cache_hashed_directories branch August 6, 2023 16:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants