twindle-co / twindle

Twindle - an open source project for beginners. Converting twitter threads to pdf, epub, and mobi format to be read by Kindle.
https://www.twindle.co
MIT License
134 stars 133 forks source link

[UPDTATED WITH USAGE] ES Modules, without any build step #734

Closed PuruVJ closed 3 years ago

PuruVJ commented 3 years ago

Let's add ES Modules to our code!

What?

Yes. Let's add ES Modules to our code.

Why?

ES Modules have been the de-facto module system of the web for the last 5 years, but Node hasn't caught up yet. It still uses the old require method, which you can hook up with destructuring to get something like this.

const { readFile, writeFile } = require('fs/promises')

And its fine, but it gets really aweful when you have something like this:

// Entry program
require("./helpers/logger");
require("dotenv").config();
const { getCommandlineArgs, prepareCli } = require("./cli");
const Renderer = require("./renderer");
const { getTweetsById, getTweetsFromArray } = require("./twitter");
const { getOutputFilePath } = require("./utils/path");
const { sendToKindle } = require("./utils/send-to-kindle");
const { getTweetIDs } = require("./twitter/scraping");
const { UserError } = require("./helpers/error");
const { red } = require("kleur");
const { isValidEmail } = require("./utils/helpers");

All these above are fine, but there isn't any clear separation what can be imports and what can be a simple variable. You could sneak const x = sum(1, 2) on the 4th line, and you could very easily skip it while skimming the code.

Now look at the example with imports

import "dotenv/config";
import { red } from "kleur";
import { getCommandlineArgs, prepareCli } from "./cli";
import { UserError } from "./helpers/error";
import "./helpers/logger";
import Renderer from "./renderer";
import { getTweetsById, getTweetsFromArray } from "./twitter";
import { getTweetIDs } from "./twitter/scraping";
import { isValidEmail } from "./utils/helpers";
import { getOutputFilePath } from "./utils/path";
import { sendToKindle } from "./utils/send-to-kindle";

So much cleaner. We humans read from left to read(English), and the first thing on the left starts with an import, making it extremely clear that this is an import, rather than following along to see the require.

It would make code navigation in the code much easier too(VSCode)

It also enables Top level await, which is total win.

Enables 'strict mode' by default, so no cheating๐Ÿ˜‰!

The Dinosaur ๐Ÿฑโ€๐Ÿ‰ in the room

My other concern is Deno. Deno is a runtime package just like NodeJS, runs using V8 engine like Node, but is much more futuristic and awesome. It doesn't support require, only imports.

It's still quite young, but in 1-2 years, its most probably going to become as much a [developer]household name as Node itself. Already hundreds of thousands of packages have been ported to it, and over a million it already supports, thanks to Skypack CDN(previously Pika pkg), and github itself.

Now, Deno support is nowhere in our current pipeline, and probably won't be for some time, but already having Imports will reduce the migration time significantly, and if we can already ship as a CLI for Deno, it would be better for our image in the Open Source community as a universal and progressive program(Or CLI, as you would call it)

How

We will use a NPM package called esm. It's not a bundler, its a runtime dependency, just like kleur or puppeteer we're using already. Program will run when you enter node index.js, just like how it would run without esm.

No build step. No extra unnecessary config.

Here's how it goes.

Step 1

Install esm npm install esm -D

Step 2

Rename twindle-cli/index.js to twindle-cli/main.js

Step 3

Create twindle-cli/index.js, and to it:

// index.js
require = require('esm')(module)
module.exports = require('./main.js')

And that's it! You have working ES Modules implemented in just 5 minutes

Questions

Wouldn't this introduce a build step?

Nope! Read the explanation above in the How section

Is it beginner friendly?

ES Imports are much more beginner-friendly, as a lot of beginners who'll be joining this project would be more knowledgeable about Imports (We JS devs are constantly screaming "Imports" on twitter all the time) than about require.

So by switching to ES Modules, we'll actually be making the codebase more accessible to the beginners

Will it require a lot of rewrite?

As a matter of fact, it won't. There are amazing extensions on VSCode that allow you to change all the require into imports with just a single Shortcut key combo.

Older Node version support

During runtime, esm converts all the modules, on-the-fly, into CommonJS modules, making them compatible with as far back as the first version of NodeJS

Is it fast?

Our project size is not very big (and most probably won't be, it's a simple project ๐Ÿ™‚), and NodeJS is extremely extremely fast, so no, introducing esm simply won't make it slow. Even on serverless, where there's the very irritating paradigm of Cold Starts

Mix and match imports and requires?

Sometimes you would need to use both import and require like this:

import puppeteer from "puppeeter";
const { writeFile } = require("fs").promises;

This will work completely. As esm says in their features, image

How much time would this refactoring take?

No more than 1 hour. Can be done in half hour

Conclusion

Imports and strict mode will give those of us actively working on the code a lot of breathing room.

What are your thoughts @johnjacobkenny @Mira-Alf @proful @pranavgoel29 @shekhar10feb ?

johnjacobkenny commented 3 years ago

Today I learnt about esm from reading your article. I think that the codebase is evolving way too fast for newbies to understand. The typings are now added to all functions and I don't know how easy it is for beginners to understand it. We might as well add TS and babel support at this stage lol :rofl:

If we are to stay true to our original idea of being beginner friendly, we should take some inputs from the beginners too to understand how we might bridge the gap for them to be able to understand the code. This would mean more resources related to this change that would enable them to understand.

As a techie I am for this change. As a team member I am concerned that others are being left behind.

I'm rambling here :sweat_smile:

I would like to see others' thoughts @twindle-co/developer

PuruVJ commented 3 years ago

@johnjacobkenny We'll have to think when to do it and how to do it. Using these tools will add the complexity of having to keep one terminal open for watch, and the other to test. That's not much problem if the user is simply running the code. But for beginners, running npm start, then opening another terminal and start testing would be quite radical. We can always introduce the option to compile on every node ., though that wouldn't be the most optimal solution.

We'd have to increase the complexity slowly.

I love Typescript and use it everywhere, but typescript would be a tad too much, as it's just not comments anymore, but weird characters after the consts and functions. And the current codebase, when used with VSCode, works entirely like Typescript(thanks to // @ts-check in every file).

I'm all in for babel. We'd have to introduce it sometime if we're to make the codebase scalable. And if we introduce it, we can think about Typescript then, which make it even more scalable and error free.

For now, I'm in favour of simply adding esm. Later, when we add babel, all we'd have to do is delete 1 file, rename another, and boom! Everything works perfectly. Then we can slowly introduce optional chaining (entities?.urls) and the assignment operators too (&&=, ||=) to make everything cleaner.

UnevenCoder commented 3 years ago

I am all for it , seems pretty cool :)

Mira-Alf commented 3 years ago

I am all for it. Coming from a statically typed language like Java background, I hate Javascript for the way it abuses its types. But one step at a time. I am fine with going ahead with this..

johnjacobkenny commented 3 years ago

I am all for it. Coming from a statically typed language like Java background, I hate Javascript for the way it abuses its types. But one step at a time. I am fine with going ahead with this..

Yeah I'm sure many things looked alien. But it has its pros and cons :smiley:

tolgaerdonmez commented 3 years ago

I learned this from your article and sounds awesome. I think we could go with this instead of using babel to compile our code. Nice write @PuruVJ

Proful commented 3 years ago

@PuruVJ I am accepting this extra dependency as many favors it ๐Ÿ˜‚

Raise PR with esm and refactored code.

After the changes, I assume there is no change how we run it

node . -i 1326340799148290050

PuruVJ commented 3 years ago

Not possible with esm, as jest simply doesn't work with it. Closing

johnjacobkenny commented 3 years ago

Doesn't jest have any esm mode