Skip to content

Added Content-Type for .wasm files#2

Open
byerlyb20 wants to merge 3 commits into
masterfrom
wasm-content-type
Open

Added Content-Type for .wasm files#2
byerlyb20 wants to merge 3 commits into
masterfrom
wasm-content-type

Conversation

@byerlyb20

Copy link
Copy Markdown
Contributor

No description provided.

@UnknownJoe796 UnknownJoe796 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Automated Review (commit 4bf2bdb)## PR #2: Added Content-Type for .wasm files — Review

Overall a solid change that replaces the module's default MIME types with an explicit map (adding .wasm support) and optionally creates a robots.txt. One concrete issue found.


1. depends_on uses deprecated string syntax

File: s3.tf:114
What: depends_on = ["local_file.robots-txt"] uses a quoted string reference.
Why it matters: In Terraform 1.x (which this repo requires via required_version = "~> 1.0"), quoted depends_on references are deprecated and produce warnings. They may stop working in a future major version. The correct syntax is an unquoted resource reference.
Suggested fix:

depends_on = [local_file.robots-txt]

Confidence: High


2. local_file writes into var.dist_folder as a side effect of terraform apply

File: s3.tf:63-67
What: The local_file.robots-txt resource creates a file inside the user's dist/build output directory.
Why it matters: This is a Terraform-managed resource writing into what is presumably a build artifact directory. If the dist folder is rebuilt (e.g., npm run build) between terraform apply runs, the robots.txt will be deleted by the build step and Terraform will need to recreate it. More subtly, if users run their build after terraform apply, the robots.txt could be wiped. This coupling between Terraform state and a build directory can lead to confusing drift. Consider documenting this behavior or using an S3-only approach (uploading the robots.txt directly as an aws_s3_object rather than writing it to disk).
Confidence: Medium — this works correctly as-is, but the design may surprise users.

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.

2 participants