Aliased parameters not parsed correctly with apostrophe followed by a whitespace

[RoelVB]

Major Version

4.x

Minor Version Number

3.x & 4.x

Target environment

All

Additional environment details

N/A

Expected or Desired Behavior

The aliased parameters parsing is a very nice feature of this library, but there's an issue when the parameter contains an apostrophe followed by a whitespace.

Observed Behavior

This is parsed just fine: /something('!@p1::value')

But this gets cut of at the whitespace: /something('!@p1::value with'' apostrophe and space')

Steps to Reproduce

This is a regex101.com example of the current situation. https://regex101.com/r/DIigHz/1

My suggested fix: https://regex101.com/r/cgsSEA/1

Note the negative lookbehind ((?<!')) inserted in the expression. The only concern here could be browser support. Depends on which browsers ages you still want to support.

Also. I'm not sure about the /something('param='!@p1::value'') example. Is this the actual syntax we would get? I added it based on this line: https://github.com/pnp/pnpjs/blob/61df3d1220dd09cc7030534d7db18ff89be24112/packages/sp/spqueryable.ts#L83 Or should it be /something(param='!@p1::value')?

[juliemturner]

We're happy to support a PR for this update. I would suggest, as you've likely seen, that the logic for this is fairly complicated and if we make changes and they break other people then that will have to be supported. I would add that expanding the testing might go hand and hand with this type of PR - https://github.com/pnp/pnpjs/blob/version-4/test/sp/alias.ts

[RoelVB]

Hi @juliemturner

I've looked into this some more. I think something like /something('!@p1::value','!@p2::value2') should match both @p1 and @p2 I assume? Can you confirm / do you agree?

[patrick-rodgers]

What happens when you test it? I thought it handled multiple but perhaps not. I'll be honest, this isn't an area the core team is looking to invest heavily. We welcome any PRs/improvements of course!

[RoelVB]

@patrick-rodgers No, multiples aren't currently (correctly) supported. This one doesn't work: /something('!@p1::value','!@p2::value2'). And this does: /something('!@p1::value','dummy','!@p2::value2')

I have found multiple scenarios that are not supported by the current implementation. Going to try to come up with a PR and a lot of tests.

Additional question. I haven't been able to find any clear documentation about possible structures for those URLs. Does anyone known some? Is this for example something we could encounter /something('!@p1::value', 'param=value') or would it actually be /something('!@p1::value', param=value) or both?

[RoelVB]

@juliemturner Thank you for the merge.

If I create a PR for v3, will you also merge (and release) this? Pretty please 🥺

[juliemturner]

Can you explain why you would need that? V3, as we've explained is deprecated and its our intent to only support bug fixes. IMHO this wasn't a bug but an enhancement, but the bigger point here is what is keeping you from using v4?

[RoelVB]

For me transitioning to v4 will be very labor intensive. I'm using some quite complex wrapper classes and extensions.

In my case I really see it as a bug. Because currently I'm unable to upload certain filenames. For example, currently I can't upload something like this 'is' my file.txt. I would be able to upload this filename without using an alias, but this will throw the error The length of the URL for this request exceeds the configured maxUrlLength value when we reach a certain path length.

If you don't want to update v3 anymore I will create a local patch for it.

[juliemturner]

I can release a version of v3 with this but cannot do it until the next version of v4 goes out because whatever you release last becomes the @latest version.

[github-actions[bot]]

This issue is locked for inactivity or age. If you have a related issue please open a new issue and reference this one. Closed issues are not tracked.