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

SinonFakeServer.respondWith not working with query params on 17.0.2 #2596

Closed
1bberto opened this issue May 7, 2024 · 5 comments
Closed

SinonFakeServer.respondWith not working with query params on 17.0.2 #2596

1bberto opened this issue May 7, 2024 · 5 comments

Comments

@1bberto
Copy link

1bberto commented May 7, 2024

Describe the bug
the respondWith seems to not work when the URL has a ?

To Reproduce
Steps to reproduce the behavior:

I have this

const url =  `/api/FakeApi?hash=10101050&fakeData=true&isGet=true`;

const method = 'GET';

const response = [
  status,
  { 'Content-type': 'application/json', ...headers },
  JSON.stringify(body),
];

server.respondWith(method, url, response);

the code is breaking when server.respondWith is being called

the error is

TypeError: Unexpected MODIFIER at 12, expected END
at mustConsume (webpack://my-company/./node_modules/sinon/pkg/sinon-esm.js?:19288:15)
at parse (webpack://sensei/./node_modules/sinon/pkg/sinon-esm.js?:19346:9)
at stringToRegexp (webpack://my-company/./node_modules/sinon/pkg/sinon-esm.js?:19503:27)
at pathToRegexp (webpack://my-company/./node_modules/sinon/pkg/sinon-esm.js?:19581:12)
at Object.respondWith (webpack://my-company/./node_modules/sinon/pkg/sinon-esm.js?:17830:57)

Context (please complete the following information):

  • Sinon version : 17.0.2
  • Runtime: Node v20.11.1, Chrome Headless 124.0.6367.119 (Windows 10)
@itchyny
Copy link

itchyny commented May 10, 2024

We've been blocked by this issue, too. I noticed that the issue is caused by sinonjs/nise#216. Rewriting all the usages of respondWith is a pain for us, but reading sinonjs/nise#216, this seems to be an intended breaking change.

@fatso83
Copy link
Contributor

fatso83 commented May 10, 2024

Thanks for providing this. This bug is totally on me. I am not sure how I missed James' very detailed explanations in his PR, which spells out how this is a breaking change and actually asks for input (which I also missed, must have been late).

My suggestion:

  • Update the NPM registry to point to 17.0.1 as latest and 17.0.2 as deprecated
  • Add support to Nise for re-writing expressions including query parameters to the new form through a compatibility flag
  • Make a patch version of Sinon 17 use that flag by default to restore current behavior
  • Release Sinon 18 with that flag disabled and note the requirement in the docs, but with the possibility of enabling it manually to avoid rewriting expressions

Thoughts, @43081j or @mroderick ?

@fatso83
Copy link
Contributor

fatso83 commented May 13, 2024

An interim "fix" is that I rolled the latest tag to point to 17.0.1. Wiping node_modules should be sufficient.

@43081j
Copy link
Contributor

43081j commented May 13, 2024

i think that makes sense. let me know if you want any help sorting the new flag out etc

doesn't sound too difficult. basically default it to legacy behaviour which auto-escapes ? under the hood

@fatso83
Copy link
Contributor

fatso83 commented May 15, 2024

The code repro is unfortunately not runnable (suggest using RunKit next time), so I have no 100% assurance this is fixed, but it should be fixed in the changes James shipped to Nise (enabling the legacy routes flag by default) should mean this is gone. Please test Sinon 18 and report back if this is not fixed.

@fatso83 fatso83 closed this as completed May 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants