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

Typescript poc #3334

Merged
merged 191 commits into from
Oct 25, 2020
Merged

Typescript poc #3334

merged 191 commits into from
Oct 25, 2020

Conversation

marcalexiei
Copy link
Member

Description:

Hi Chris,
We (@marcalexiei & @giovannipiller) have been thinking to contribute a bit to the project and had a couple of ideas.

Convert Ractive to Typescript

As suggested in #3172. This could help a lot while refactoring the code or hunting for bugs.

From our initial tests it looks like having a project which mixes JS and TS is possible, which should make things easier for us. We only noticed problems when importing JS from a TS file, but not the other way around.

Automate generation of API documentation

Documentation seems to be kind of an issue.
Storing part of it alongside the code should make it easier to maintain, reduce duplication and avoid inconsistencies (ex. changing a type in the code should also update the doc)


We’ve set up this PR to start some progress and gather some discussion.

To generate the documentation:

  1. npm i
  2. npm run doc

As a proof of concept, we added some content forRactive.prototype.toHTML() method and Ractive.prototype.observe, which will be found in the following paths:

  • {RepoFolder}/docs/modules/ractive_prototype_observe.html
  • {RepoFolder}/docs/modules/ractive_prototype_tohtml.html

Is breaking:

Nothing, but it's a WIP

@evs-chris
Copy link
Contributor

Just wanted to say that I think that a gradual move to ts is a great idea. There are some really chewy parts of ractive that will probably fight typing pretty hard, but it can probably be persuaded. I think it may help to modernize the build system a bit too, since gobble is no longer developed and likely stuck at node 8.

It's great to see a pr open up here! Let me know if you have any questions about the wild and wooly code base. I'd be happy to chat, and I'll try to lend a hand if you'd like when my schedule opens up a bit.

@marcalexiei
Copy link
Member Author

Hi @evs-chris, 
refactor is nearly complete but there are some points that I would like to bring to your attention:

General information

  • I have created a RactiveDefinition file which define Ractive as a class.
    
Should we consider making Ractive as a class?
  • There are many any keywords which cause many eslint warning. 
I would like to leave them and progressively remove them in the future.
In the meantime, I added --quiet option on lint:all task to exclude them from the build process log
  • All implementation change due to this refactor have a comment TSRChange followed by a briefly description of the change
  • I updated node to version 12 (ESlint no longer supports node 8)

Improvements To-Do

  • It would be nice to generate ts definition file automatically.
  • Update build system
  • Automatic documentation generation
  • Some Ractive prototype function may use generic typings to improve their param signature

Do you want to procede with a merge to a 2.x branch or would you like to make some adjustments?

@giovannipiller
Copy link
Contributor

Hi everyone, just as a FYI, next week I'm planning to run a few tests using a build of this branch on a pretty complex web-app.
As of now I've been able to build and run everything without any obvious issue.

I'll report back next week!

@evs-chris
Copy link
Contributor

Nice! I think this would be a good opportunity to start a 2.x branch.

I personally think a factory may be a better pattern than a class, but I'm not positive. Going into 2.0, we have the opportunity to adjust some things.

@marcalexiei would you like to be added as a maintainer?

On a somewhat related topic, I cleaned up the rollup-plugin-ractive-bin project a bit last week, and it now integrates much more cleanly with the typescript rollup plugin. What do you usually use to build your projects?

@marcalexiei
Copy link
Member Author

marcalexiei commented Oct 8, 2020

I personally think a factory may be a better pattern than a class, but I'm not positive. Going into 2.0, we have the opportunity to adjust some things.

Yes, I imagine that there is some way to combine factory pattern and types.
In my opinion the main goal is to find a way to have automatic definition generation.
Any solution that makes this possibile is a good to go.

@marcalexiei would you like to be added as a maintainer?

Yes. Thank you 😊

What do you usually use to build your projects?

I use webpack with a custom HTML loader which parse HTML into a template object (basically a fork from official webpack-html-loader).
Basically for each component there are 3 files:

  • component.rhtml -> which is parsed by the custom loader
  • component.css
  • component.js -> import the two other files and export the Component "definition"

@marcalexiei
Copy link
Member Author

Hi @evs-chris, do you want to make other adjustments before merging into a new branch?

Just re-checking 😀

@giovannipiller
Copy link
Contributor

I'm having a few issues while using the runtime.js bundle of Ractive. I'll check them out with @marcalexiei and report back.

@evs-chris evs-chris changed the base branch from dev to 2.0 October 23, 2020 19:17
@evs-chris
Copy link
Contributor

No, I think we should probably go ahead and merge this into the new 2.0 branch and carry on from there!

- fix spelling of Bracked in Bracket
- improve extractRefs signature
- add markdownlint to validate md files
  - validation is performed via vscode extension
- fix md files to meet markdownlint rules
- remove unused jsconfig.json
@marcalexiei marcalexiei added this to the 2.0 milestone Oct 25, 2020
@marcalexiei marcalexiei marked this pull request as ready for review October 25, 2020 08:50
@marcalexiei marcalexiei merged commit e832206 into ractivejs:2.0 Oct 25, 2020
@marcalexiei marcalexiei deleted the typescript-poc branch October 25, 2020 09:19
@marcalexiei
Copy link
Member Author

As a next step I will try to remove as many "any" as possible from the source code.

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.

4 participants