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

Refactor rehype-mathjax #45

Merged
merged 16 commits into from
May 30, 2020
Merged

Refactor rehype-mathjax #45

merged 16 commits into from
May 30, 2020

Conversation

wooorm
Copy link
Member

@wooorm wooorm commented May 8, 2020

Builds upon GH-44 but:

  • use functions instead of classes, removing some duplicated code (shaving 2kb of build, which isn’t much for SVG, 😅 but goes from 3kb to 1kb for the browser plugin!)
  • uses fixtures instead of depending on mathjax calls
  • fix stylesheet injection on complete documents (it was added after </html>)
  • remove unused dependencies
  • fix files in package.json
  • rewrite readme

/cc @nzt, @Ir1d would appreciate a review!

@wooorm wooorm added the 🏡 area/internal This affects the hidden internals label May 8, 2020
@wooorm wooorm requested a review from Rokt33r May 8, 2020 10:44
@codecov-io

This comment has been minimized.

@tani
Copy link
Contributor

tani commented May 8, 2020

This comment is not request to update code. I'd like to ask you why you prefer to use ES5 syntax? For example, you write const packages = require('mathjax-full/js/input/tex/AllPackages').AllPackages instead of const {packages: AllPackages} = require('mathjax-full/js/input/tex/AllPackages'). I'm interested in your coding style.

@wooorm
Copy link
Member Author

wooorm commented May 8, 2020

I'm interested in your coding style.

So this project wasn’t originally made by me, so it isn’t 100% using “my code style”. There are some consts! 😅 But most projects in unified do indeed follow my code style. I hope you don’t mind me refactoring the code you wrote!

We are currently discussing this btw!

I have a couple of thoughts about this. First: the most important thing is consistency. When I’m in a project that uses double quotes, or tabs, or semicolons, I follow those rules!

Second: When building an app, on your own or with a team, pick whatever style you want! But as this is tiny open source modules that are running on many (sometimes old) computers that I don‘t control, I prefer making them work as much as possible (otherwise people become angry and raise issues 😉)

Third, I started coding before Babel, ES5, or browser code on npm. When JavaScript had a single way to create a function, and a single way to create a variable. When the world was simpler (except for all the browser bugs) 😅 I still prefer that in most cases unless a) there is a much better way to do something, or b) I’m writing a script or an app.
I feel that not using the newer features leads to:

  • consistency (you can’t do everything with arrow functions, you sometimes need normal functions)
  • less bugs (developers love new shiny features, such as arrow functions, but then maybe don’t understand that they can’t access this anymore)
  • readability (functions have a name! they have the word function before them!)
  • less discussion (there’s only one choice: named functions!)
  • less change (it takes a lot of time to update hundreds of projects every year: if they work fine, why change to newer syntax?)
  • less tools (no compiler means code runs directly and that npm install is faster)

I’m really interested in hearing your perspective on this! Is the way I write code readable to you?

@tani
Copy link
Contributor

tani commented May 29, 2020

@wooorm Thank you for nice explanation. I agree your points but we have another way to satisfy your request with programming paradigm.

  • Immutable object (mutable variables infect unexpected behaviors)
  • State less (It is impossible to know all possible states of Object)
  • this free (this is quite different from other object system, because JavaScript is not class-based object system and is prototype-based object system)
  • That is pure functional programming style.

You and me love UNIX philosophy and KISS (Keep it Simple, Stupid). What does simple mean? Is it simple of syntax for you? It is simple of semantics for me. Biggest JavaScript community is moving to that paradigm, pure functional programming style. For example, React, Immutable.js. ECMAScript committee does also implement purely functional features in the specification, for instance, allow function, const declaration, syntax sugar of Object.assign.

The syntax is important. Less tools is important. The readability is important. I totally agree with you. Although, syntax and tools is not so related to the making bugs directly because valid syntax works perfectly except misunderstandings of developer ;). We cannot validate of semantics without our efforts like TypeScript, eslint, const declaration as it is impossible to see the semantics physically, which is different from the syntax.

That's why I prefer to the simple of semantics than the simple of syntax.

ANYWAY, I appreciate your gentle explanation, and LGTM ❤️

If no one comment to this code anymore, we should merge this code.

@codecov-commenter

This comment has been minimized.

@wooorm wooorm merged commit a652a78 into master May 30, 2020
@wooorm wooorm deleted the mathjax branch May 30, 2020 08:41
@wooorm
Copy link
Member Author

wooorm commented May 30, 2020

Released! Awesome ✨ Thanks @nzt for making rehype-mathjax!

@wooorm
Copy link
Member Author

wooorm commented May 30, 2020

And thanks @Ir1d for the package name. Hope this one, with the browser plugin, will work well!

@wooorm wooorm added 🦋 type/enhancement This is great to have 🧑 semver/major This is a change labels May 30, 2020
@Ir1d
Copy link

Ir1d commented May 30, 2020

Thank you guys for your excellent work 😘 This is much better than m y previous implementation!

@wooorm wooorm added the 💪 phase/solved Post is done label Aug 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏡 area/internal This affects the hidden internals 💪 phase/solved Post is done 🧑 semver/major This is a change 🦋 type/enhancement This is great to have
Development

Successfully merging this pull request may close these issues.

5 participants