Automattic / jetpack

Security, performance, marketing, and design tools — Jetpack is made by WordPress experts to make WP sites safer and faster, and help you grow your traffic.
https://jetpack.com/
Other
1.59k stars 798 forks source link

General: Introduce TypeScript to the monorepo #16375

Open jeherve opened 4 years ago

jeherve commented 4 years ago

Branching off from #16363

I suppose this could be the opportunity to introduce TypeScript to Jetpack, but if we do so I'd rather we do in a separate PR, by starting with introducing the necessary linting plugins and updating our tools to take into account the new file type.

To introduce TypeScript, we'd need to make a few changes.

Here is what comes to mind right now, but part of this work should be to make sure everything is up to date.

manzoorwanijk commented 2 years ago
  • Add coding guidelines, or refer to the Calypso coding guidelines from Calypso:

coding-guidelines.md already refers to the mentioned document. Is there anything else we want to do?

jeherve commented 2 years ago

Sorry, that wasn't super clear.

We have different documents in the monorepo, used to help guide new contributors when contributing to specific parts of Jetpack. Here are 2 examples:

In my mind, when introducing TypeScript, we would update one or more of those documents to encourage / offer some general recommendations as to when and where to use TypeScript, and when not to. A bit like we did in #16353 when we decided to start moving towards function components instead of classes in the Jetpack dashboard.

We do not have to add an extensive section to our own docs: we could add a small section that gives some general advice and redirect to the guidelines in the Calypso repo if one wants to find out more.

All that said, I do not know where we should start introducing TypeScript today. We already use it in the Boost plugin, but we do not have coding guidelines docs there afaik. Where should we introduce TypeScript next, if we should introduce it somewhere in the first place? In the Jetpack dashboard? In Jetpack RNA components? In Jetpack blocks? Somewhere else?

cc @oskosk; what's your take on this?

manzoorwanijk commented 2 years ago

All that said, I do not know where we should start introducing TypeScript today. We already use it in the Boost plugin, but we do not have coding guidelines docs there afaik. Where should we introduce TypeScript next, if we should introduce it somewhere in the first place? In the Jetpack dashboard? In Jetpack RNA components? In Jetpack blocks? Somewhere else?

Apart from Boost, we already have TypeScript working examples in:

I don't know how blocks are managed and built, so I don't know if we need any extra configuration for that area.

manzoorwanijk commented 2 years ago

These changes are need in projects/plugins/jetpack/tools/webpack.config.extensions.js to allow TypeScript in blocks

--- a/projects/plugins/jetpack/tools/webpack.config.extensions.js
+++ b/projects/plugins/jetpack/tools/webpack.config.extensions.js
@@ -8,6 +8,7 @@
 const fs = require( 'fs' );
 const CopyWebpackPlugin = require( 'copy-webpack-plugin' );
 const jetpackWebpackConfig = require( '@automattic/jetpack-webpack-config/webpack' );
+const jetpackWebpackUtils = require( '@automattic/jetpack-webpack-config/utils' );
 const path = require( 'path' );
 const webpack = jetpackWebpackConfig.webpack;
 const StaticSiteGeneratorPlugin = require( 'static-site-generator-webpack-plugin' );
@@ -38,9 +39,9 @@ const noop = function () {};
 function presetProductionExtensions( type, inputDir, presetBlocks ) {
        return presetBlocks
                .flatMap( block =>
-                       blockEditorDirectories.map( dir => path.join( inputDir, dir, block, `${ type }.js` ) )
+                       blockEditorDirectories.map( dir => path.join( inputDir, dir, block, type ) )
                )
-               .filter( fs.existsSync );
+               .filter( jetpackWebpackUtils.moduleExists );
 }

 const presetPath = path.join( __dirname, '../extensions', 'index.json' );
@@ -57,8 +58,8 @@ const presetBetaBlocks = [ ...presetExperimentalBlocks, ...( presetIndex.beta ||

 // Helps split up each block into its own folder view script
 const viewBlocksScripts = presetBetaBlocks.reduce( ( viewBlocks, block ) => {
-       const viewScriptPath = path.join( __dirname, '../extensions/blocks', block, 'view.js' );
-       if ( fs.existsSync( viewScriptPath ) ) {
+       const viewScriptPath = path.join( __dirname, '../extensions/blocks', block, 'view' );
+       if ( jetpackWebpackUtils.moduleExists( viewScriptPath ) ) {
                viewBlocks[ block + '/view' ] = [ viewSetup, ...[ viewScriptPath ] ];
        }
        return viewBlocks;
// projects/js-packages/webpack-config/src/utils.js

const fs = require( 'fs' );
const { resolve } = require( './webpack' );

const moduleExists = modulePath => {
    return (
        fs.existsSync( modulePath ) ||
        resolve.extensions.some( ext => fs.existsSync( `${ modulePath }${ ext }` ) )
    );
};

module.exports = {
    moduleExists,
};
jeherve commented 2 years ago

Internal reference: p9dueE-4XW-p2

This gives us the answer we were looking for; TypeScript will first be used in RNA components, and we'll develop guidelines there based on our experience. We can then extend that work to other parts of the repo based on our experience.

kraftbj commented 1 year ago

Internal conversation about ts coding standards: p1695284862244749-slack-C02JJ910CNL