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

TS code examples #10990

Merged
merged 62 commits into from
Jun 11, 2024
Merged

TS code examples #10990

merged 62 commits into from
Jun 11, 2024

Conversation

sequba
Copy link
Contributor

@sequba sequba commented May 21, 2024

Context

  • convert all JS examples to TS by hand
    • accessibility/
    • cell-features/
    • cell-types/
    • formulas/
    • navigation/
    • rows/
    • tools-and-building/
    • accessories-and-menus/
    • cell-functions/
    • columns/
    • getting-started/
    • internationalization/
    • optimization/
    • security/
    • technical-specification/
    • upgrade-and-migration/
  • create a script that
    • compiles all TS examples files to JS (tsc)
    • formats them to be more human-readable (eslint)
  • run the script during the docs build

There are 192 js files in the docs/content/guides dir:

$ find docs/content/guides -name "*.js" | wc
    192     192   13076

How has this been tested?

This has been tested locally plus using visual tests for regression checking

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature or improvement (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Additional language file or change to the existing one (translations)

Related issue(s):

  1. https://github.com/handsontable/dev-handsontable/issues/1867

Affected project(s):

  • handsontable
  • @handsontable/angular
  • @handsontable/react
  • @handsontable/vue
  • @handsontable/vue3

Checklist:

[skip changelog]

@sequba sequba requested a review from evanSe May 21, 2024 13:23
Copy link

github-actions bot commented May 22, 2024

Launch the local version of documentation by running:

npm run docs:review fdf2eb9e6cbad6a13ca28529a634f925a6746a07

evanSe
evanSe previously approved these changes May 27, 2024
Copy link
Member

@evanSe evanSe left a comment

Choose a reason for hiding this comment

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

In general everything looks good, a few improvements I would create.

  • generate multiple examples files at once (currently it takes along time)
  • do have generation as part of start script if its so slow.

If speeding up the script takes more than an hour I wouldn't bother with it

@evanSe
Copy link
Member

evanSe commented May 27, 2024

Also its worth rebasing this branch, it has 165 commits currently.

@sequba sequba force-pushed the feature/dev-issue-1867-without-1905 branch from 7064854 to 5095c10 Compare May 29, 2024 13:07
@sequba sequba requested a review from evanSe May 29, 2024 13:26
@sequba
Copy link
Contributor Author

sequba commented May 29, 2024

Rebased. @evanSe please re-review.

@sequba sequba self-assigned this May 29, 2024
evanSe
evanSe previously approved these changes Jun 2, 2024
@magierg
Copy link
Contributor

magierg commented Jun 6, 2024

@sequba There is a problem when running npm run docs:start as TS -> JS conversion not being triggered

@sequba
Copy link
Contributor Author

sequba commented Jun 10, 2024

@evanSe wrote here:

do have generation as part of start script if its so slow.

@magierg wrote here:

There is a problem when running npm run docs:start as TS -> JS conversion not being triggered

Currently, npm run docs:start doesn't run the generate-js-examples script, but it could be added there. Personally, I prefer the start script to be fast, as we use it frequently during development. @evanSe, please make a decision about it.

@evanSe evanSe force-pushed the feature/dev-issue-1867-without-1905 branch from 2031e7d to 9830081 Compare June 11, 2024 07:52
@sequba sequba merged commit 369d7dd into develop Jun 11, 2024
22 checks passed
@sequba sequba deleted the feature/dev-issue-1867-without-1905 branch June 11, 2024 11:53
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.

None yet

4 participants