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: escape function path regex chars #1686

Merged
merged 3 commits into from
Aug 16, 2022
Merged

Conversation

Skye-31
Copy link
Contributor

@Skye-31 Skye-31 commented Aug 16, 2022

Closes #1685

Currently, if you have a file such as functions/*.ts, pages attempts to run path-to-regex on this, and this will fail, as it assumes it's a regex character (it shouldn't be). This results in an Unexpected MODIFIER at 1, expected END. error. This PR correctly escapes these characters to solve the issue (confirmed fix from Alastair)

@changeset-bot
Copy link

changeset-bot bot commented Aug 16, 2022

🦋 Changeset detected

Latest commit: 8938b36

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

@Skye-31 Skye-31 added the pages Relating to Pages label Aug 16, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Aug 16, 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/2868686305/npm-package-wrangler-1686

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

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

Or you can use npx with this latest build directly:

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

@Skye-31 Skye-31 force-pushed the fix-pages-dev-unexpected-modifier branch from b4dbd18 to 4175bd4 Compare August 16, 2022 12:12
@Skye-31 Skye-31 marked this pull request as draft August 16, 2022 12:22
@Skye-31 Skye-31 force-pushed the fix-pages-dev-unexpected-modifier branch from 4175bd4 to bab249a Compare August 16, 2022 12:29
@codecov
Copy link

codecov bot commented Aug 16, 2022

Codecov Report

Merging #1686 (8938b36) into main (bec170c) will increase coverage by 0.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1686      +/-   ##
==========================================
+ Coverage   80.88%   80.89%   +0.01%     
==========================================
  Files          91       91              
  Lines        6068     6068              
  Branches     1556     1556              
==========================================
+ Hits         4908     4909       +1     
+ Misses       1160     1159       -1     
Impacted Files Coverage Δ
...ackages/wrangler/src/__tests__/helpers/mock-bin.ts 100.00% <0.00%> (+5.26%) ⬆️

@Skye-31 Skye-31 marked this pull request as ready for review August 16, 2022 12:42
@Skye-31 Skye-31 force-pushed the fix-pages-dev-unexpected-modifier branch from bab249a to 69aa904 Compare August 16, 2022 13:41
@jahands
Copy link
Contributor

jahands commented Aug 16, 2022

Hey Skye! Thanks for putting this together.
Not quite sure I understand what this is fixing. Some files have * in them?

Would you mind adding some test cases before this change (with working paths) to ensure we don't break existing functionality? Then we could add this change with additional tests showing the old + additional paths work.

Edit: I see now we have some tests - per our convo, maybe add some tests with _ and -, and any other character's folks might use. Thanks!

@Skye-31 Skye-31 merged commit a0a3ffd into main Aug 16, 2022
@Skye-31 Skye-31 deleted the fix-pages-dev-unexpected-modifier branch August 16, 2022 14:32
@github-actions github-actions bot mentioned this pull request Aug 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pages Relating to Pages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 BUG: pages dev router does not escape regex characters correctly.
2 participants