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

Should this plugin also replace "default" with the field name? #1

Closed
felipecrs opened this issue Mar 5, 2021 · 12 comments
Closed

Should this plugin also replace "default" with the field name? #1

felipecrs opened this issue Mar 5, 2021 · 12 comments
Labels
bug Something isn't working

Comments

@felipecrs
Copy link

First of all: your plugin saved my life. Thank you so much!

This started at TypeStrong/typedoc#1521. So, now that all modules are merged, I think it does not make sense anymore to use "default" as the fieldname.

So, my point here is: should this plugin also replace "default" with the field name? I'm not asking to do, just discussing :).

image

@felipecrs
Copy link
Author

PS: this makes the API not browsable, as only one "default" link works.

@krisztianb
Copy link
Owner

krisztianb commented Mar 5, 2021

Hi Felipe. I'm glad the plugin is useful for somebody.

If you look at the code you will see that the plugin does nothing fancy. It only moves all reflections from the modules into the project.

So the answer to your question is: No, it should not rename any exports at the moment. Naming collisions are not handled. I can have a look at that though.

What would be your expected output?

@krisztianb krisztianb added the enhancement New feature or request label Mar 5, 2021
@felipecrs
Copy link
Author

No, it should not rename any exports at the moment. Naming collisions are not handled.

I thought more about it, and I agree, you're right. Especially because users may want to do the "renaming" without using your plugin together, so it would be better to be a standalone plugin.

What would be your expected output?

With TypeDoc 0.19 and --mode files, this is what I had:

image

Now, with TypeDoc 0.20 and your plugin:

image

Without your plugin:

image

(despite the bad naming, they at least don't overlap)

PS: I'm using docusaurus, but I already tested and this behavior remains in vanilla TypeDoc html generator.

@krisztianb
Copy link
Owner

That would mean that moving the reflections to the project somehow destroys their "path". I'll take a look when I find the time. This should by fairly easy to reproduce with a simple example.

@krisztianb krisztianb added bug Something isn't working and removed enhancement New feature or request labels Mar 5, 2021
@felipecrs
Copy link
Author

If you want, you can try with my repository:

https://github.com/felipecrs/megatar/tree/upgrade-typedoc/website
https://gitpod.io/#https://github.com/felipecrs/megatar/tree/upgrade-typedoc/website

@felipecrs
Copy link
Author

@krisztianb
Copy link
Owner

Interesting. Thanks for the heads up. I was thinking about adding an option (something like renameDefaults: true|false) to this plugin so that you don't need to create a separate one, since when merging modules this can be a common problem.

@felipecrs
Copy link
Author

felipecrs commented Mar 6, 2021

The main motivation for having a separate plugin is to be able to use it without your plugin.

@felipecrs
Copy link
Author

I mean, for those who would still like to have their defaults renamed but they don't want to merge the modules. But the code itself is quite easy, as proposed by Gerrit0 at TypeStrong/typedoc#1521 (comment).

@felipecrs
Copy link
Author

felipecrs commented Mar 7, 2021

I thought in these options:

  • As you said, add the code and an option (and think about leaving the renameDefaults: true it by default, as it would cause the docs to overlap otherwise)
  • Add typedoc-plugin-rename-defaults as dependency of yours, but I don't really know if that would work
  • Simply document in the README, so who find this issue know what to do (install the typedoc-plugin-rename-defaults plugin)

@krisztianb
Copy link
Owner

I think renameDefaults: true makes a lot of sense. I don't think anybody wants to have something called default and moreover risk overwriting reflections when merging modules. I think I can add this option sometime next week.

@krisztianb
Copy link
Owner

New option mergeModulesRenameDefaults added in version 2.0.0. 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants