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

migrated code with 0 completed tests #23

Open
wants to merge 5 commits into
base: next
Choose a base branch
from

Conversation

SlimTimDotPng
Copy link

I managed to migrated the code base as you asked but i think there is a chance that the karma framework does not work with typescript. I had some issues with building the program but i managed to fix it today. Let me know if this is sufficient.

@tuplle tuplle changed the base branch from master to next November 14, 2024 22:29
@tuplle
Copy link
Owner

tuplle commented Nov 14, 2024

I changed the target branch to next where I migrated to Rollup from Webpack and to Jest testing framework from Karma.
With this setup, it should be easier to build the project with typescript sources and then test it.

Please synchronize your branch and resolve the conflicts that this migration brought.

Here are some links for help on how to build a typescript with rollup:

package.json Outdated
"directories": {
"test": "test"
},
"dependencies": {
Copy link
Owner

Choose a reason for hiding this comment

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

Why there are so many dependencies when the library actually uses nothing from this? 🤔
The library should be as clean as possible, so most of the dependencies should be dev dependencies.

* @param word - The word to insert.
* @param data - Optional data to map to the word.
*/
insert(word: string, data?: any): boolean {
Copy link
Owner

Choose a reason for hiding this comment

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

This class could be defined with a generic type (i.e. Trie<T>) so data in a trie can have a type.
The type any can be avoided this way.

https://www.typescriptlang.org/docs/handbook/2/generics.html

* If the node is the end of a word, it has the flag `isEndOfWord` set to true and `word` is set to the indexed word.
* If the node is the end of a word, it can contain some additional data.
*/
export default class TrieNode {
Copy link
Owner

Choose a reason for hiding this comment

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

If you make Trie a class with generic type you need to also change it here to TrieNode<T> because here data is actually stored.

@SlimTimDotPng
Copy link
Author

SlimTimDotPng commented Nov 17, 2024

It should be fixed now. When i run it using "npx jest" the tests pass.

@SlimTimDotPng
Copy link
Author

I accidentally closed the RP so i'll reopen it.

@SlimTimDotPng SlimTimDotPng reopened this Nov 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants