Skip to content
This repository has been archived by the owner on Aug 3, 2023. It is now read-only.

Allow non-Webpack Workers Sites #1833

Merged
merged 3 commits into from
Apr 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/settings/toml/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ const WATCH_DIR: &str = "src";
#[derive(Clone, Debug, Deserialize, PartialEq, Serialize)]
#[serde(deny_unknown_fields)]
pub struct Builder {
command: Option<String>,
pub command: Option<String>,
#[serde(default = "project_root")]
pub cwd: PathBuf,
#[serde(default = "upload_dir")]
Expand Down
31 changes: 25 additions & 6 deletions src/settings/toml/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,11 +312,30 @@ impl Manifest {
environment_name: Option<&str>,
preview: bool,
) -> Result<Target, failure::Error> {
// Site projects are always webpack for now; don't let toml override this.
let target_type = match self.site {
Some(_) => TargetType::Webpack,
None => self.target_type.clone(),
};
if self.site.is_some() {
match self.target_type {
TargetType::Rust => {
failure::bail!(format!(
"{} Workers Sites does not support Rust type projects.",
emoji::WARN
))
}
TargetType::JavaScript => {
let error_message = format!(
"{} Workers Sites requires using a bundler, and your configuration indicates that you aren't using one. You can fix this by:\n* setting your project type to \"webpack\" to use our automatically configured webpack bundler.\n* setting your project type to \"javascript\", and configuring a build command in the `[build]` section if you wish to use your choice of bundler.",
emoji::WARN
);
if let Some(build) = &self.build {
if build.command.is_none() {
failure::bail!(error_message)
}
} else {
failure::bail!(error_message)
}
}
_ => {}
}
}

/*
From https://developers.cloudflare.com/workers/cli-wrangler/configuration#keys
Expand All @@ -327,7 +346,7 @@ impl Manifest {
Not inherited: Must be defined for every environment individually.
*/
let mut target = Target {
target_type, // Top level
Copy link
Contributor

Choose a reason for hiding this comment

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

top level is correct -- we don't let you override target_type in environments

target_type: self.target_type.clone(), // Top level
account_id: self.account_id.clone(), // Inherited
webpack_config: self.webpack_config.clone(), // Inherited
build: self.build.clone(), // Inherited
Expand Down
18 changes: 9 additions & 9 deletions src/upload/form/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,14 @@ pub fn build(
}
}

if let Some(asset_manifest) = asset_manifest {
log::info!("adding __STATIC_CONTENT_MANIFEST");
let binding = "__STATIC_CONTENT_MANIFEST".to_string();
let asset_manifest_blob = get_asset_manifest_blob(asset_manifest)?;
let text_blob = TextBlob::new(asset_manifest_blob, binding)?;
text_blobs.push(text_blob);
}

match target_type {
TargetType::Rust => {
log::info!("Rust project detected. Publishing...");
Expand Down Expand Up @@ -89,7 +97,7 @@ pub fn build(
log::info!("Plain JavaScript project detected. Publishing...");
let package_dir = target.package_dir()?;
let package = Package::new(&package_dir)?;
let script_path = package.main(&package_dir)?;
let script_path = package_dir.join(package.main(&package_dir)?);
Copy link
Contributor

Choose a reason for hiding this comment

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

why the change here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Without it, script_path is relative to the entrypoint. By joining package_dir with script_path, we get an absolute path, which is needed since wrangler actually executes in the project directory (parent).

So yes, it's needed, otherwise wrangler can't find that script to deploy.


let assets = ServiceWorkerAssets::new(
script_path,
Expand Down Expand Up @@ -185,14 +193,6 @@ pub fn build(
wasm_modules.push(wasm_module);
}

if let Some(asset_manifest) = asset_manifest {
log::info!("adding __STATIC_CONTENT_MANIFEST");
let binding = "__STATIC_CONTENT_MANIFEST".to_string();
let asset_manifest_blob = get_asset_manifest_blob(asset_manifest)?;
let text_blob = TextBlob::new(asset_manifest_blob, binding)?;
text_blobs.push(text_blob);
}

let assets = ServiceWorkerAssets::new(
script_path,
wasm_modules,
Expand Down