Fix docs for UrlSearchParams - #400

[ktiedt]

Type: bug

The following has been addressed in the PR:

• [x ] There is a related issue
• [ x] All code has been formatted with prettier as per the readme code style guidelines

Description: Update docs for UrlSearchParams Resolves #400

[jsf-clabot]

All committers have signed the CLA.

[bitranch]

The code for UrlSearchParams is only using export default. I'm guessing it should be using both export and export default to match the way pretty much all the other dojo2 packages do exports. If that was fixed, the documentation would be correct (actually correct either way).

[nicknisi]

I think you're both correct. Using the default export in the docs makes them more consistent and adding a named export to the code makes it more consistent, too. @ktiedt would you mind adding a named export to UrlSearchParams?

[ktiedt]

I wouldn't argue that change if its more consistent with the other modules.

So to be clear, keep the docs change, and add the named export? Or update the export and lose the doc update (minus the corrections to the wrong code).

Edit: Looking at other modules, there are quite a few that are the same as UrlSearchParams and quite a few that are as you have described...

[ktiedt]

While waiting on clarification here, I went through each of the Constructor classes in dojo/core, the results...

Almost all of them do what UrlSearchParam does currently: https://github.com/dojo/core/blob/master/src/DateObject.ts#L61 https://github.com/dojo/core/blob/master/src/IdentityRegistry.ts#L34 https://github.com/dojo/core/blob/master/src/List.ts#L10 https://github.com/dojo/core/blob/master/src/MatchRegistry.ts#L15 https://github.com/dojo/core/blob/master/src/MultiMap.ts#L11 https://github.com/dojo/core/blob/master/src/Scheduler.ts#L19 https://github.com/dojo/core/blob/master/src/async/ExtensiblePromise.ts#L37 https://github.com/dojo/core/blob/master/src/async/Task.ts#L36 https://github.com/dojo/core/blob/master/src/request/Headers.ts#L13 https://github.com/dojo/core/blob/master/src/request/ProviderRegistry.ts#L5 https://github.com/dojo/core/blob/master/src/request/SubscriptionPool.ts#L3 https://github.com/dojo/core/blob/master/src/request/TimeoutError.ts#L1

Most of these do not have a correlating /doc/ file, but with this in mind, if the expectation is to provide default and named exports, I would propose limiting this PR and #400 to fixing the documentation errors in UrlSearchParam's docs and I'd file a 2nd ticket for bringing these files exports into consistency.... My one concern here is, if dojo/core is this disparate, how bad are the other repos as well?

[nicknisi]

Good catch. Yeah, this repo is out of sync with the other Dojo 2 repos, where it is standard to have a named and default export. dojo/core is older than most other repos, and the standard was adopted later and not reapplied here. Let's go with your proposal and use this PR/issue to sync up URLSearchParams and then we can create another issue for the rest of the dojo/core modules. Thanks for looking into this!

So, we can leave the docs changes you made and add the named export to URLSearchParams.

[ktiedt]

The last commit, puts the docs back to import {UrlSearchParams} and fixes the documentation error, as well as adding the default export and named export in.

[ktiedt]

I also opened https://github.com/dojo/core/issues/404 to address the other Constructor exports

[nicknisi]

Thanks for the contribution!