Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Continue JS -> TS conversion #435

Merged
merged 13 commits into from
Oct 9, 2023
Merged

Conversation

Mersho
Copy link
Contributor

@Mersho Mersho commented Oct 5, 2023

No description provided.

@Mersho
Copy link
Contributor Author

Mersho commented Oct 6, 2023

I am investigating the CI issue. Feel free to offer me any suggestions.

Mersho added 9 commits October 6, 2023 15:28
Also replace "require" with "import" and
"module.exports" with "export".
We need to enable esModuleInterop to solve error TS1192:
```
error TS1192: Module '"tls"' has no default export.
```
The languageModel & fiatModel interface created to type-check
the data and ensure that it conforms to
the expected structure and format.
This can help you avoid errors and bugs when
working with the data in your code.
This commit enables resolveJsonModule to import module
with '.json' extension.
The --downlevelIteration flag is a TypeScript compiler
option that enables support for iterating over new
concepts like Map, Set, or Generator in older JavaScript
runtimes. By default, TypeScript targets ES3, which does
not support these features. If you use a for...of loop or
a spread operator on an iterable object, you may get an error.
Use Date instead of Date.toISOString cause paid_at has type
Date and during the conversion from js to ts,
we got compilation errors.
@Mersho Mersho force-pushed the convert-js-squashed branch from b8836a0 to c1cf28c Compare October 6, 2023 12:05
@grunch
Copy link
Member

grunch commented Oct 6, 2023

I am investigating the CI issue. Feel free to offer me any suggestions.

thanks for this great contribution, not a clue on the ci issue, if it's too hard to find the ci issue maybe splitting this PR would be a good way to know where is the problem

@Mersho Mersho force-pushed the convert-js-squashed branch 2 times, most recently from 8d58871 to dea18cd Compare October 9, 2023 10:30
Mersho added 4 commits October 9, 2023 15:17
Using null instead of a boolean/undefined type is better.
I had to change the '|', otherwise typescript would
complain this error msg:
```
The '|' operator is not allowed for boolean types.
Consider using '||' instead.
```
@Mersho
Copy link
Contributor Author

Mersho commented Oct 9, 2023

My attempts to change regex to solve exponential backtracking failed, so I am reverting the commits.

@Mersho Mersho force-pushed the convert-js-squashed branch 2 times, most recently from 13981f5 to 068e439 Compare October 9, 2023 11:54
@Mersho
Copy link
Contributor Author

Mersho commented Oct 9, 2023

This PR #436 helped me identify the problem and should also be merged. CI is green, everything seems to be fine.

Copy link
Member

@grunch grunch left a comment

Choose a reason for hiding this comment

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

LGTM

@grunch
Copy link
Member

grunch commented Oct 9, 2023

This PR #436 helped me identify the problem and should also be merged. CI is green, everything seems to be fine.

This PR #436 helped me identify the problem and should also be merged. CI is green, everything seems to be fine.

it looks great, thanks for this great job 👏🏽

@grunch grunch merged commit c4b2406 into lnp2pBot:main Oct 9, 2023
grunch added a commit that referenced this pull request Oct 19, 2023
grunch added a commit that referenced this pull request Oct 19, 2023
Mersho added a commit to Mersho/bot that referenced this pull request Oct 26, 2023
Mersho added a commit to Mersho/bot that referenced this pull request May 15, 2024
This commit completes the TypeScript conversion [1] by replacing
the CommonJS `require` syntax with TypeScript's `import` syntax.

[1]: lnp2pBot#435
grunch pushed a commit that referenced this pull request May 16, 2024
This commit completes the TypeScript conversion [1] by replacing
the CommonJS `require` syntax with TypeScript's `import` syntax.

[1]: #435
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants