[ ] 1. The issue provides a reproduction available on GitHub, Stackblitz or CodeSandbox
Make sure to fork this template and run yarn generate in the terminal.
Please make sure the GraphQL Scalars package version under package.json matches yours.
[ ] 2. A failing test has been provided
[ ] 3. A local solution has been provided
[ ] 4. A pull request is pending review
Describe the bug
The scalar URL adds a trailing slash to URLs, which is undesirable as it modifies the input URL string. The reason this happens is because it uses the native URL class, which "normalizes" the URL string:
// Outputs https://test.test/
new URL("https://test.test").href;
It is undesirable for URL "normalization" to occur. Some servers respond with different content depending if there is a trailing slash or not. For example, these URLs have different redirects/responses:
In our API, we have situations where we want the URL to be preserved exactly as it is input/stored, as the purpose of the API is to test what certain URLs respond with. A customer might want to separately test the difference between their website rendering a route with a slash and without, but if the scalar URL normalizes both URL strings to the same string with a trailing slash then they can't do that.
To Reproduce
Steps to reproduce the behavior:
Try resolving the value https://test.test with a field with the scalar URL, and see that the value in the resolved data becomes https://test.test/.
Expected behavior
The scalar URL should validate the URL string, but not "normalize" the string if it's a valid URL.
Environment:
OS:
GraphQL Scalars Version:
NodeJS:
Additional context
As a side note, there are opportunities to optimize this scalar URL implementation. It would be good that if it receives a value that is already a URL instance, it uses that instead of inefficiently converting it to a string and then back again into another URL instance:
Also when serializing, if it's already a URL instance it should just serialize that instance via .href instead of converting to a string then back into a URL instance, then back into a string again:
Issue workflow progress
Progress of the issue based on the Contributor Workflow
Describe the bug
The scalar
URLadds a trailing slash to URLs, which is undesirable as it modifies the input URL string. The reason this happens is because it uses the nativeURLclass, which "normalizes" the URL string:https://github.com/Urigo/graphql-scalars/blob/3182ad51178f2306f120c844deb2de7eae03e3ba/src/scalars/URL.ts#L18
It is undesirable for URL "normalization" to occur. Some servers respond with different content depending if there is a trailing slash or not. For example, these URLs have different redirects/responses:
In our API, we have situations where we want the URL to be preserved exactly as it is input/stored, as the purpose of the API is to test what certain URLs respond with. A customer might want to separately test the difference between their website rendering a route with a slash and without, but if the scalar
URLnormalizes both URL strings to the same string with a trailing slash then they can't do that.To Reproduce Steps to reproduce the behavior:
Try resolving the value
https://test.testwith a field with the scalarURL, and see that the value in the resolved data becomeshttps://test.test/.Expected behavior
The scalar
URLshould validate the URL string, but not "normalize" the string if it's a valid URL.Environment:
Additional context
As a side note, there are opportunities to optimize this scalar
URLimplementation. It would be good that if it receives a value that is already aURLinstance, it uses that instead of inefficiently converting it to a string and then back again into anotherURLinstance:https://github.com/Urigo/graphql-scalars/blob/3182ad51178f2306f120c844deb2de7eae03e3ba/src/scalars/URL.ts#L18
Also when serializing, if it's already a
URLinstance it should just serialize that instance via.hrefinstead of converting to a string then back into aURLinstance, then back into a string again:https://github.com/Urigo/graphql-scalars/blob/3182ad51178f2306f120c844deb2de7eae03e3ba/src/scalars/URL.ts#L10-L16
Furthermore, the
URLinstance method.toString()shouldn't be used when.hrefcan be used to the same effect with less code.