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 incorrect extension extraction from file paths #1045

Merged
merged 2 commits into from
May 19, 2022

Conversation

jrf0110
Copy link
Member

@jrf0110 jrf0110 commented May 17, 2022

Fixes #1029

Our extension extraction logic was taking into account folder names, which can include periods. The logic would incorrectly identify a file path of .well-known/foo as having the extension of well-known/foo when in reality it should be an empty string.

@changeset-bot
Copy link

changeset-bot bot commented May 17, 2022

🦋 Changeset detected

Latest commit: 00e06e6

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

This PR includes changesets to release 1 package
Name Type
wrangler 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

@github-actions
Copy link
Contributor

github-actions bot commented May 17, 2022

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

npm install --save-dev https://prerelease-registry.developers.workers.dev/runs/2350676635/npm-package-wrangler-1045

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

npm install --save-dev https://prerelease-registry.developers.workers.dev/prs/1045/npm-package-wrangler-1045

Or you can use npx with this latest build directly:

npx https://prerelease-registry.developers.workers.dev/runs/2350676635/npm-package-wrangler-1045 dev path/to/script.js

@jrf0110 jrf0110 force-pushed the 1029-folders-periods branch from d679919 to fd1a5f8 Compare May 17, 2022 16:55
Copy link
Contributor

@threepointone threepointone left a comment

Choose a reason for hiding this comment

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

Looks good to me. Would you like to add a test for this?

@jrf0110 jrf0110 force-pushed the 1029-folders-periods branch from fd1a5f8 to 5ba5255 Compare May 17, 2022 19:28
Copy link
Contributor

@threepointone threepointone left a comment

Choose a reason for hiding this comment

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

I'm happy to stamp this, but would you also like to add a changeset for this? Run npx changeset at the root, choose wrangler / patch, and the content can be exactly the title + description of this PR.

Copy link
Contributor

@GregBrimble GregBrimble left a comment

Choose a reason for hiding this comment

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

Thanks @jrf0110 , appreciate ya!

@jrf0110 jrf0110 force-pushed the 1029-folders-periods branch from 5ba5255 to 6833987 Compare May 18, 2022 15:19
…t assets publish

Fixes cloudflare#1029

Our extension extraction logic was taking into account folder names, which can include periods. The logic would incorrectly identify a file path of `.well-known/foo` as having the extension of `well-known/foo` when in reality it should be an empty string.
@jrf0110 jrf0110 force-pushed the 1029-folders-periods branch from 6833987 to 0b536ee Compare May 18, 2022 15:32
Copy link
Contributor

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

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

LGTM

@threepointone threepointone merged commit 8eeef9a into cloudflare:main May 19, 2022
@github-actions github-actions bot mentioned this pull request May 19, 2022
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.

🐛 BUG: Issue uploading a file without an extension to a Pages project
4 participants