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

jsdoc types processing added, refs #88. #97

Merged
merged 4 commits into from
Jul 24, 2024
Merged

jsdoc types processing added, refs #88. #97

merged 4 commits into from
Jul 24, 2024

Conversation

Sozialarchiv
Copy link
Contributor

With this pr a types file gets automatically generated during build.

@Sozialarchiv
Copy link
Contributor Author

Sozialarchiv commented Jul 22, 2024

If you think the types are not specific enough (It is admittedly more of a workaround) I see two alternatives:

  1. Migrate the project to typescript. I am willing to create a pr.
  2. Create manuelle a types file. Personally, I think the first part is technically cleaner (danger of divergence), but I also understand if the project should remain js.

@neSpecc
Copy link
Contributor

neSpecc commented Jul 22, 2024

If you think the types are not specific enough (It is admittedly more of a workaround) I see two alternatives:

  1. Migrate the project to typescript. I would willing to create a pr.
  2. Create a types file. Personally, I think the first part is technically cleaner (danger of divergence), but I also understand if the project should remain js.

Your workaround is fine. Just update package.json to specify types file. Check this example: https://github.com/editor-js/header/blob/master/package.json#L19-L25

@Sozialarchiv
Copy link
Contributor Author

Your workaround is fine. Just update package.json to specify types file. Check this example: https://github.com/editor-js/header/blob/master/package.json#L19-L25

A line with the types path should be already included in package.json. Or am I misunderstanding something?
https://github.com/editor-js/list/blob/bfa3e388978fc9fa1cc887c83fdfe89741b34045/package.json#L22

@neSpecc
Copy link
Contributor

neSpecc commented Jul 22, 2024

A line with the types path should be already included in package.json. Or am I misunderstanding something?

https://github.com/editor-js/list/blob/bfa3e388978fc9fa1cc887c83fdfe89741b34045/package.json#L22

You should also declare "types" property on the root level

@Sozialarchiv
Copy link
Contributor Author

You should also declare "types" property on the root level

Thanks a lot. I added it.

Copy link
Contributor

@neSpecc neSpecc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

update a patch version, please

@Sozialarchiv
Copy link
Contributor Author

update a patch version, please

Done.

@Sozialarchiv
Copy link
Contributor Author

Is there anything else to fix?

@neSpecc neSpecc merged commit 2977364 into editor-js:master Jul 24, 2024
@neSpecc
Copy link
Contributor

neSpecc commented Jul 24, 2024

Thanks, @Sozialarchiv. Nice job. If you want to continue contributing, let me know

@Sozialarchiv
Copy link
Contributor Author

@neSpecc

Thanks, @Sozialarchiv. Nice job. If you want to continue contributing, let me know

If you agree I would like to add this types-patch to the other core plugins without types. It is imho a pragmatic way to solve the missing types and it is wished by several people: codex-team/editor.js#2660

In addition my plan is to solve some smaller issues which were complained about by our users. There are already pull requests for some of them. I will test and give feedback there.

@Sozialarchiv
Copy link
Contributor Author

Sozialarchiv commented Jul 25, 2024

I have seen that some plugins are already written in typescript. Alternatively, I can also make typescript migrations if desired. However, such migrations can lead to merge conflicts with existing pull requests, which may initially speak in favor of the jsdoc workaround.

@neSpecc
Copy link
Contributor

neSpecc commented Jul 25, 2024

@neSpecc

Thanks, @Sozialarchiv. Nice job. If you want to continue contributing, let me know

If you agree I would like to add this types-patch to the other core plugins without types. It is imho a pragmatic way to solve the missing types and it is wished by several people: codex-team/editor.js#2660

In addition my plan is to solve some smaller issues which were complained about by our users. There are already pull requests for some of them. I will test and give feedback there.

Let's add such patch to the Table block.

@Sozialarchiv
Copy link
Contributor Author

Let's add such patch to the Table block.

editor-js/table#151

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants