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

Implemented preserve_file_names config #4742

Merged
merged 8 commits into from
Feb 23, 2024

Conversation

benycodes
Copy link
Contributor

@benycodes benycodes commented Jan 11, 2024

Fixes #4741

preserveFileNames in createModuleCollector

Author has addressed the following:

@benycodes benycodes requested a review from a team as a code owner January 11, 2024 20:03
Copy link

changeset-bot bot commented Jan 11, 2024

🦋 Changeset detected

Latest commit: 4df213f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
wrangler Minor
@cloudflare/vitest-pool-workers Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link
Contributor

github-actions bot commented Jan 15, 2024

A wrangler prerelease is available for testing. You can install this latest build in your project with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8019695860/npm-package-wrangler-4742

You can reference the automatically updated head of this PR with:

npm install --save-dev https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/prs/4742/npm-package-wrangler-4742

Or you can use npx with this latest build directly:

npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8019695860/npm-package-wrangler-4742 dev path/to/script.js
Additional artifacts:
npx https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8019695860/npm-package-create-cloudflare-4742 --no-auto-update
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8019695860/npm-package-cloudflare-kv-asset-handler-4742
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8019695860/npm-package-miniflare-4742
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8019695860/npm-package-cloudflare-pages-shared-4742
npm install https://prerelease-registry.devprod.cloudflare.dev/workers-sdk/runs/8019695860/npm-package-cloudflare-vitest-pool-workers-4742

Note that these links will no longer work once the GitHub Actions artifact expires.


wrangler@3.29.0 includes the following runtime dependencies:

Package Constraint Resolved
miniflare workspace:* 3.20240208.0
workerd 1.20240208.0 1.20240208.0
workerd --version 1.20240208.0 2024-02-08

Please ensure constraints are pinned, and miniflare/workerd minor versions match.

@petebacondarwin
Copy link
Contributor

Needs some formatting fixes, it seems.

@penalosa
Copy link
Contributor

Thanks for adding this! It would be great to see some tests around this behaviour, if possible

@penalosa penalosa added the awaiting reporter response Needs clarification or followup from OP label Jan 22, 2024
@lrapoport-cf
Copy link
Contributor

hi @benycodes :) can you please fix the formatting and add tests per @petebacondarwin and @penalosa 's comments above? else we'll close the PR until you're able to revisit. thanks!

@mrbbot mrbbot added awaiting Cloudflare response Awaiting response from workers-sdk maintainer team and removed awaiting reporter response Needs clarification or followup from OP labels Feb 7, 2024
@penalosa penalosa added awaiting reporter response Needs clarification or followup from OP and removed awaiting Cloudflare response Awaiting response from workers-sdk maintainer team labels Feb 19, 2024
@petebacondarwin
Copy link
Contributor

Checking with @admah whether this is something we want to take on and get across the finish line ourselves.

@benycodes
Copy link
Contributor Author

Formatting issues should be fixed now.

Copy link
Contributor Author

@benycodes benycodes left a comment

Choose a reason for hiding this comment

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

Tests added

@petebacondarwin
Copy link
Contributor

Thanks for fixing the formatting and adding tests.
We need to create a PR to update the docs and also add a changeset to this PR. I'll do these now...

@petebacondarwin
Copy link
Contributor

There was actually a small bug in the filename processing, I think I have fixed it.

Copy link

codecov bot commented Feb 23, 2024

Codecov Report

Attention: Patch coverage is 80.00000% with 1 lines in your changes are missing coverage. Please review.

Project coverage is 70.37%. Comparing base (64236b0) to head (4df213f).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4742      +/-   ##
==========================================
+ Coverage   70.33%   70.37%   +0.04%     
==========================================
  Files         298      298              
  Lines       15512    15515       +3     
  Branches     3983     3987       +4     
==========================================
+ Hits        10910    10919       +9     
+ Misses       4602     4596       -6     
Files Coverage Δ
packages/wrangler/src/config/validation.ts 89.70% <ø> (ø)
packages/wrangler/src/deploy/deploy.ts 89.74% <100.00%> (+0.02%) ⬆️
...rangler/src/deployment-bundle/module-collection.ts 87.87% <75.00%> (+0.18%) ⬆️

... and 6 files with indirect coverage changes

@petebacondarwin petebacondarwin added enhancement New feature or request and removed awaiting reporter response Needs clarification or followup from OP labels Feb 23, 2024
@petebacondarwin petebacondarwin merged commit c2f3f1e into cloudflare:main Feb 23, 2024
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🚀 Feature Request: preserveFileNames option
6 participants