1033715 - set up transpilation from JSX to JS

Status: RESOLVED FIXED

(Hello (Loop) :: Client, defect, P1)

Description

[Dan Mosedale (:dmosedale, :dmose)]

As a browser developer, it should be easy to locally build JSX files into JS and package them into the browser.

a) transpilation should happen locally and the transpiled files should be checked in, in order to avoid introducing a build-time dependency on node.js (the React compiler is written in node)

b) generated desktop and shared JS files should be packaged into the browser using jar.mn (instead of the original JS files)

c) generated standalone JS files should live somewhere under the standalone/ directory for now, since we don't yet have a sophisticated standalone deployment process

d) the workflow should be documented in the README

Ideally, this would be invocable using the normal browser tools (eg "mach build-jsx" or something) for browser developer comfort.  But I don't think that's crazy high value, so if the fast path here is just to write a simple Python script, that seems OK too.

Comment 1

[Andrei Oprea [mention me on [e-mail]]]

Attachment: Add build script to transpile JSX files to JS

Comment 2

[Andrei Oprea [mention me on [e-mail]]]

Comment on attachment 8450631 [details] [diff] [review]
Add build script to transpile JSX files to JS

># HG changeset patch
># User Andrei Oprea <[e-mail]>
>
>Bug 1033715 - Add build script to transpile JSX files to JS
>
>diff --git a/browser/components/loop/jsx-build b/browser/components/loop/jsx-build
>new file mode 100755
>index 0000000..909375b
>--- /dev/null
>+++ b/browser/components/loop/jsx-build
>@@ -0,0 +1,85 @@
>+#! /usr/bin/env python
>+
>+import os
>+from distutils import spawn
>+import subprocess
>+from threading import Thread
>+import argparse
>+
>+src_files = [] # files to be compiled
>+loop_path = './browser/components/loop/'
>+
>+# search for react-tools install
>+jsx_path = spawn.find_executable('jsx')
>+if jsx_path is None:
>+    print 'You do not have the react-tools installed'
>+    print 'Please do $ sudo npm install -g react-tools'
>+    exit(1)
>+
>+# parse the CLI arguments
>+description = 'Loop build tool for JSX files. ' + \
>+              'Will scan entire loop directory and compile them in place'
>+parser = argparse.ArgumentParser(description=description)
>+parser.add_argument('--watch', '-w', action='store_true', help='continuous' + \
>+                                    'build based on file changes (optional)')
>+args = parser.parse_args()
>+
>+# find all .jsx files
>+for dirname, dirnames, filenames in os.walk('.'):
>+    for filename in filenames:
>+        if '.jsx' == filename[-4:]:
>+            f = os.path.join(dirname, filename)
>+            src_files.append((dirname, filename))
>+
>+# loop through all tuples and get unique dirs only
>+unique_dirs = []
>+for src_file in src_files:
>+    directory = src_file[0] # (directory, filename)
>+    if directory not in unique_dirs:
>+        unique_dirs.append(directory)
>+
>+def jsx_run_watcher(path):
>+    subprocess.call(['jsx', '-w', '-x', 'jsx', path, path])
>+
>+def start_jsx_watcher_threads(dirs):
>+    """
>+        starts a thread with a jsx watch process
>+        for every dir in the dirs argument
>+    """
>+    threads = []
>+    for udir in dirs:
>+        t = Thread(target = jsx_run_watcher, args = (udir,))
>+        threads.append(t)
>+        t.start()
>+
>+    for t in threads:
>+        t.join()
>+
>+def check_file_packaging(src_files):
>+    """
>+        get all lines from jar.mn file
>+        check against the files we are going to compile
>+    """
>+    # get all lines from jar.mn
>+    packaged_files = [line.strip() for line in open('./jar.mn')]
>+
>+    # loop through our compiled files and compare against jar.mn
>+    missing_jar_files = [] # initially empty
>+    for compiled_file in src_files:
>+        fname = compiled_file[1][0:-1] # *.jsx -> *.js
>+        if not any(fname in x for x in packaged_files):
>+            missing_jar_files.append(fname)
>+
>+    # output a message to the user
>+    if len(missing_jar_files):
>+        for f in missing_jar_files:
>+            print f + ' not in jar.mn file'
>+
>+check_file_packaging(src_files)
>+
>+if (args.watch):
>+    start_jsx_watcher_threads(unique_dirs)
>+else:
>+    for d in unique_dirs:
>+        out = subprocess.call(['jsx', '-x', 'jsx', d, d])
>+

Comment 3

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 8450631 [details] [diff] [review]
Add build script to transpile JSX files to JS

Stealing back the review here.

Comment 4

[Mark Banner (:standard8)]

(In reply to Dan Mosedale (:dmose - needinfo? me for responses) from comment #3)
> Stealing back the review here.

Thanks. Sorry for not getting to it yet. I did have a glance the other day - my main concern that I haven't written down was making sure that it moved the jsx files as well (although admittedly the panel patch hadn't landed when this was generated).

Also, we should update the readme about this script & the jsx -> js files that we're doing.

Comment 5

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 8450631 [details] [diff] [review]
Add build script to transpile JSX files to JS

Review of attachment 8450631 [details] [diff] [review]:
-----------------------------------------------------------------

This seems to work quite nicely so far!  Here's a first set of comments; I've addressed these in my tree already.  There are a few more things to sort out; let's discuss F2F.

::: browser/components/loop/jsx-build
@@ +26,5 @@
> +
> +# find all .jsx files
> +for dirname, dirnames, filenames in os.walk('.'):
> +    for filename in filenames:
> +        if '.jsx' == filename[-4:]:

if '.jsx' == os.path.splitext(filename)[1]:

is slightly cleaner.

@@ +31,5 @@
> +            f = os.path.join(dirname, filename)
> +            src_files.append((dirname, filename))
> +
> +# loop through all tuples and get unique dirs only
> +unique_dirs = []

Calling this unique_jsx_dirs would make it a bit more clear.

@@ +37,5 @@
> +    directory = src_file[0] # (directory, filename)
> +    if directory not in unique_dirs:
> +        unique_dirs.append(directory)
> +
> +def jsx_run_watcher(path):

PEP 8 wants to blank lines between functions.

@@ +54,5 @@
> +
> +    for t in threads:
> +        t.join()
> +
> +def check_file_packaging(src_files):

This src_files shadows the one in the global scope, which can lead to confusion for code readers.  Suggest changing this to srcs.

Comment 6

[Dan Mosedale (:dmosedale, :dmose)]

Attachment: Add build script to transpile JSX files to JS, r=dmose

Comment 7

[Dan Mosedale (:dmosedale, :dmose)]

Comment on attachment 8451934 [details] [diff] [review]
Add build script to transpile JSX files to JS, r=dmose

Review of attachment 8451934 [details] [diff] [review]:
-----------------------------------------------------------------

Additional cleanups addressed here, r=dmose for this version of the patch.

Comment 8

[Dan Mosedale (:dmosedale, :dmose)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/999a107ad069

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/999a107ad069

Comment 10

[u279076]

Untracking for QA. Please needinfo me to request specific testing.