stylus icon indicating copy to clipboard operation
stylus copied to clipboard

Add build system

Open eight04 opened this issue 3 years ago • 20 comments

@tophf Since it is so important that we can't add new js/css files to the page without it. I'd like to know what do we expect from a build system PR.

  1. Just find a tool that can concatenate CSS/JS on an HTML page automatically? Maybe we can try parcel and start from non-shared modules.
  2. Convert the page into individual components (svelte?), still have to exclude shared modules until all pages have been converted.
  3. Start building the project structure from scratch? Build the project in another branch and eventually use it as the main?
  4. Other plans?

eight04 avatar Dec 11 '21 06:12 eight04

1 (automatic concatenating) should be sufficient. Content scripts, too. Might add minifying but only if it makes the page load noticeably faster.

2 is for the future, I don't know svelte yet.

3 will be necessary within a year for a ManifestV3 rewrite because there will be quite a lot of unique code in bg.

Edit:

  • It might be nice to have an automatic watcher to rebuild when developing, maybe even with HMR (hot-module-reload).
  • We might have to move source code inside src, although maybe not, and build in dist.

tophf avatar Dec 11 '21 09:12 tophf

I think we can build one of our extension pages first. We will have something like this:

.tx:
  ...
.github:
  ...
dist:
  ... # move the current code to this folder
src:
  options.html # suppose we start from the options page which is simpler
  options:
    options.js
    options.css
    other.files...
tools:
  ...
dotfiles
readme

When building, the build tool bundles src/*.html to extension/*.html along with its js/css files. If that works, then we move other pages (manage.html, edit.html, etc) to src.

If everything works and all files have been moved to src, then we put dist in our .gitignore.

  • It might be nice to have an automatic watcher to rebuild when developing, maybe even with HMR (hot-module-reload).

I know that web-ext is able to reload the extension automatically on file change. Don't know if HMR works in extensions.

eight04 avatar Dec 11 '21 14:12 eight04

Sounds fine overall but I hoped the build system would work seamlessly i.e. there would be no need to rework our html files except some trivial changes.

tophf avatar Dec 11 '21 14:12 tophf

Actually, I doubt it'll be helpful. Maybe for simple bundling we can write a simple build script that bundles js/css before zipping it. Whereas a proper build system should allow us to switch to imports, modules, and modern syntax like optional chaining via Babel.

tophf avatar Dec 11 '21 22:12 tophf

I think we can build one of our extension pages first.

After testing parcel for a while, I found that it doesn't fit this. By default it traverses the entry file and locate every resources. It tries to build manage.html when it sees <a href="/manage.html"> in options.html.

Maybe for simple bundling we can write a simple build script that bundles js/css

Yeah rollup probably fits better. For example, if we want to build (concat/minify) codemirror, we can create an entry file: src/edit/codemirror-bundle.js

import CodeMirror from "vendor/codemirror/lib/codemirror.js";
import "vendor/codemirror/mode/css/css.js";
import "vendor/codemirror/mode/stylus/stylus.js";
import "vendor/codemirror/addon/dialog/dialog.js";
import "vendor/codemirror/addon/edit/closebrackets.js";
import "vendor/codemirror/addon/scroll/annotatescrollbar.js";
import "vendor/codemirror/addon/search/searchcursor.js";
import "vendor/codemirror/addon/search/matchesonscrollbar.js";
import "vendor/codemirror/addon/comment/comment.js";
import "vendor/codemirror/addon/selection/active-line.js";
import "vendor/codemirror/addon/edit/matchbrackets.js";
import "vendor/codemirror/addon/fold/foldcode.js";
import "vendor/codemirror/addon/fold/foldgutter.js";
import "vendor/codemirror/addon/fold/brace-fold.js";
import "vendor/codemirror/addon/fold/indent-fold.js";
import "vendor/codemirror/addon/fold/comment-fold.js";
import "vendor/codemirror/addon/lint/lint.js";
import "vendor/codemirror/addon/hint/show-hint.js";
import "vendor/codemirror/addon/hint/css-hint.js";
import "vendor/codemirror/keymap/sublime.js";
import "vendor-overwrites/codemirror-addon/match-highlighter.js"; // we have to modify the path in `require(...)`

// assign CodeMirror to global since our other files still use globals
window.CodeMirror = CodeMirror;

Then use rollup to build it into dist/edit/codemirror-bundle.js. Then we can replace edit.html

    <script src="vendor/codemirror/lib/codemirror.js"></script>
    <script src="vendor/codemirror/mode/css/css.js"></script>
    <script src="vendor/codemirror/mode/stylus/stylus.js"></script>
    <script src="vendor/codemirror/addon/dialog/dialog.js"></script>
    <script src="vendor/codemirror/addon/edit/closebrackets.js"></script>
    <script src="vendor/codemirror/addon/scroll/annotatescrollbar.js"></script>
    <script src="vendor/codemirror/addon/search/searchcursor.js"></script>
    <script src="vendor/codemirror/addon/search/matchesonscrollbar.js"></script>
    <script src="vendor/codemirror/addon/comment/comment.js"></script>
    <script src="vendor/codemirror/addon/selection/active-line.js"></script>
    <script src="vendor/codemirror/addon/edit/matchbrackets.js"></script>
    <script src="vendor/codemirror/addon/fold/foldcode.js"></script>
    <script src="vendor/codemirror/addon/fold/foldgutter.js"></script>
    <script src="vendor/codemirror/addon/fold/brace-fold.js"></script>
    <script src="vendor/codemirror/addon/fold/indent-fold.js"></script>
    <script src="vendor/codemirror/addon/fold/comment-fold.js"></script>
    <script src="vendor/codemirror/addon/lint/lint.js"></script>
    <script src="vendor/codemirror/addon/hint/show-hint.js"></script>
    <script src="vendor/codemirror/addon/hint/css-hint.js"></script>
    <script src="vendor/codemirror/keymap/sublime.js"></script>
    <script src="vendor-overwrites/codemirror-addon/match-highlighter.js"></script>

with

<script src="edit/codemirror-bundle.js"></script>

eight04 avatar Dec 12 '21 09:12 eight04

Rollup seems promising.

tophf avatar Dec 12 '21 09:12 tophf

Here is a proof-of-concept branch: https://github.com/openstyles/stylus/tree/dev-rollup In this branch we try to bundle codemirror. First we create src/codemirror/base.mjs, which imports files that are used by both edit.html and install-usercss.js. Note that we import them from node_modules directly instead of dist/vendor: https://github.com/openstyles/stylus/blob/dev-rollup/src/codemirror/base.mjs

Then we create src/codemirror/edit.mjs, which import the previous file and other plugins that will be used in the editor: https://github.com/openstyles/stylus/blob/dev-rollup/src/codemirror/edit.mjs

In rollup.config.js, we build them to correspond location in dist. Shared chunks are put in dist/chunks. Though we don't have shared chunks in this case: https://github.com/openstyles/stylus/blob/c13be76553b111dd594b12f0181b09210c95dd57/rollup.config.mjs#L10-L17

After bundling, inject the script to HTML/js files: https://github.com/openstyles/stylus/blob/c13be76553b111dd594b12f0181b09210c95dd57/rollup.config.mjs#L23-L35

With the following result: https://github.com/openstyles/stylus/blob/c13be76553b111dd594b12f0181b09210c95dd57/dist/edit.html#L24 https://github.com/openstyles/stylus/blob/c13be76553b111dd594b12f0181b09210c95dd57/dist/install-usercss/install-usercss.js#L41-L44

If you want to test it, first BAKCUP YOUR DATA. If you have side-loaded the extension in the browser, it will be uninstalled when switching to this branch because path to manifest.json is changed.

After checkout, run npm install && npm run start-chrome to test it.

eight04 avatar Dec 12 '21 16:12 eight04

Wait. If we choose to implement the simple build system then we shouldn't move any files into src or dist, but rather produce the bundle(s) before zipping automatically, using a config for rollup or maybe even writing the bundle-writer script manually.

Moving everything into proper directories like src/dist should be postponed to the time when we implement proper build system, e.g. webpack, that provides a lot more than just bundling, which is why the added hassle is worth it.

tophf avatar Dec 12 '21 16:12 tophf

no dist

I really hope we can put extension code in a sub folder (#514). Rollup will still work without dist folder since we specify the path in the config.

we shouldn't move any files into src

I think this may work but it will be more complicated. We have to rewrite more stuff. Take dev-rollup branch for example, we have to replace <script src="vendor/codemirror... with <script src="codemirror/... at build time. We also have to add codemirror/* to zip ignores. The build script probably will be very large if we start building 90% of the extension.

Moving everything into proper directories like src/dist should be postponed to the time when we implement proper build system, e.g. webpack

By using src and dist structure, we can also make the transfer smoother.

For the bundler, I believe that everything which can be done in webpack can also be done in rollup. Since you mentioned that webpack do a lot more, do you plan to have a specific feature in the build system which is support by webpack?

eight04 avatar Dec 13 '21 05:12 eight04

For now, we can start by bundling CodeMirror. It will be a single file used both by the editor and the installer, there's no need to split it: the time wasted to load the unused addons in the installer should be [much] less than the time we gain by bundling. If necessary to simplify this task, we can remove all require for CodeMirror/addons and use normal html script/link tags instead.

Then the content scripts, which should be trivial.

In the future I also want to move to src but only when we use a full build system that allows imports, modules, modern syntax via Babel, which will be a relatively big refactor, not too complicated hopefully. I don't know if it should be webpack or rollup, so I assumed webpack since it seems to be much more popular and advanced/extensible.

tophf avatar Dec 13 '21 09:12 tophf

BTW, an alternative and hopefully a very simple method for the CodeMirror bundle is to produce it in our build-vendor script. It will be minified with sourcemaps so we can debug it. The sourcemaps won't be included in zip. Bundling CodeMirror will cut the wasted time in half (at least) so it's a worthwhile optimization even if we don't bundle our own files (but in this case we should undo/embed the 5 files added recently in your style settings PR).

tophf avatar Dec 13 '21 09:12 tophf

In the future I also want to move to src

I suppose that you meant to move all files to src in the future. I suggest to have a src folder now, but only for files that needs to be bundled, explained below.

Then the content scripts, which should be trivial

Here I use the term source for ES modules that need to be bundled e.g. https://github.com/openstyles/stylus/blob/dev-rollup/src/codemirror/edit.mjs, and the term dist for bundle output e.g. https://github.com/openstyles/stylus/blob/dev-rollup/dist/codemirror/edit.js.

Suppose we want to bundle content/apply.js. Without a src folder, we have to either

  1. Store the source file and dist file with different filename. For example, write our code in content/apply.src.js, then build apply/content.src.js to content/apply.js, and add *.src.* to ignore array in zip.js.
  2. Or, write our code in content/apply.js. When building, we copy (cp or whatever) everything but content/apply.js to another folder like dist, then bundle content/apply.js to dist/content/apply.js, then zip dist folder.

(2) is especially bad because we have to modify the exclude array (the exclude array for cp) each time adding a source file.

If there is a src folder, we can write our code in src/content/apply.js and build it to content/apply.js, then add src to ignore array in zip.js.

The difference between *.src.js v.s. src/* emerges when we bundle 90% of our files:

  1. If we use *.src.js, we will see that 90% of our repo is filled with content/apply.src.js, content/apply.js, background/background.src.js, background/background.js, etc, source and dist are mixed together.
  2. If we use src/*, we will just see that 90% of our files have been moved to src folder.

allows imports, modules, modern syntax via Babel minified with sourcemaps

Both rollup and webpack support these features.

I don't know if it should be webpack or rollup

I am not familiar with webpack so I can't answer either. Here are some features we still need to test:

  1. Can we use CSS as an entry file in rollup/webpack?
  2. Will rollup/webpack be able to generate shared CSS chunks?
    • When I tried parcel, it inlines all @import "global.css" statements, causing duplicated code in multiple minified CSS file. Pretty sure that is not what we want.

eight04 avatar Dec 14 '21 11:12 eight04

Re src, with the simple build system I see no need to move anything.

(2) is especially bad because we have to modify the exclude array (the exclude array for cp) each time adding a source file.

We haven't added or removed to the content files declaration in years so this is not a concern. You seem to be overcomplicating this. AFAICT, all we need for content files is to read manifest.json's first content_scripts item and concatenate the files into dist/content.js. No need for complicated universal rules since we need to produce just one such bundle currently.

tophf avatar Dec 14 '21 12:12 tophf

(2) is especially bad because we have to modify the exclude array (the exclude array for cp) each time adding a source file.

We haven't added or removed to the content files declaration in years so this is not a concern

I didn't mean to add/remove to the content files declaration. By "adding a source file", I mean to bundle more files using the build tool. I'll demonstrate the "exclude list" problem below.

AFAICT, all we need for content files is to read manifest.json's first content_scripts item and concatenate the files into dist/content.js.

Now we have these files in content script: js/polyfill.js, js/msg.js, js/prefs.js, content/style-injector.js, content/apply.js to dist/content.js, so actually we have to:

  1. Bundles (concat/minify) js/polyfill.js, js/msg.js, js/prefs.js, content/style-injector.js, content/apply.js to dist/content.js.

  2. Modify manifest.json, use the new path "js": ["dist/content.js"].

  3. In tools/zip.js, we have to exclude content/style-injector.js, content/apply.js.

As we bundle more files, we will add more files to tool/zip.js.

P.S. Note that with the above setup, we actually duplicated js/polyfill.js, js/msg.js, js/prefs.js. Other extension scripts still use unminified version in the js folder.

we need to produce just one such bundle currently

What do you mean by currently? Do we want to solve the problem that we can't add modular js/css to our html?

You seem to be overcomplicating ... No need for complicated universal rules

Moving stuff to src and dist actually simplifies stuff. In dev-rollup branch, we can even delete these lines: https://github.com/openstyles/stylus/blob/c13be76553b111dd594b12f0181b09210c95dd57/tools/zip.js#L10-L16 Because... they are outside of dist folder.

eight04 avatar Dec 14 '21 13:12 eight04

1,2,3

All of this sounds super trivial to me because there's no need to manually exclude anything: we'll just add the necessary logic to the script. Like I initially suggested, this is performed when zipping so it can be a part of zip.js, it'll read manifest.json and make the necessary trivial changes.

What do you mean by currently? Do we want to solve the problem that we can't add modular js/css to our html?

Bundle for our main content script entry, which is the only one that consists of multiple files.

We need to start from something. I don't like your solution in dev-rollup branch because it's a not a full build system. I suggested that we start from CodeMirror and the content scripts. For this we don't need to move anything. We'll add like 10 lines of code to build-vendor.js and zip.js, and remove 50 lines of code that loads CodeMirror in the editor/installer.

Then, when we're ready to perform a full switch to imports, modules, modern syntax and Babel, we move to src. Like I suggested it should probably happen in ManifestV3 rewrite.

tophf avatar Dec 14 '21 14:12 tophf

when we're ready to perform a full switch to imports, modules, modern syntax and Babel, we move to src. Like I suggested it should probably happen in ManifestV3 rewrite.

Does it mean that we can't add new js or css file to html before manifest v3? Or you are going to write a trivial script to bundle edit.css and edit.js?

eight04 avatar Dec 14 '21 16:12 eight04

it's a not a full build system

What is a "full build system"? Will it be better to choose the 3rd approach in the first post? (that I can build everything in another branch and switch the branch eventually.)

eight04 avatar Dec 14 '21 16:12 eight04

Any amount of separate files is fine as long as they are loaded on demand after a click. In the PR for style settings, 4 files out of 5 can be loaded like that. The fifth file is super small so it can be embedded in edit/base.js or maybe in toolbox.js if it can be reused in other places.

As for ManifestV3 rewrite, we have one year to do it before MV2 is disabled in Chrome. Unfortunately MV3 is a steaming pile of bug-infested crap at the moment, which is why we shouldn't hurry. Maybe we could start in the summer.

A full build system is just a proper/standard build system that most projects use. There will be src with imports, modules, modern syntax, automatic polyfilling via .browserslistrc, etc. There won't be a permanent dist that contains copies of our files.

tophf avatar Dec 14 '21 16:12 tophf

A full build system is just a proper/standard build system that most projects use. There will be src with imports, modules, modern syntax, automatic polyfilling via .browserslistrc, etc. There won't be a permanent dist that contains copies of our files.

I'm still not fully understand what is a "full build system".

  1. Will it be a full build system by excluding dist folder in the dev-rollup? Since rollup/webpack already supports everything else, we just have to move current code to src (probably src/static until they are built dynamically) and add dist to .gitignore.

  2. Maybe you mean that we have to convert all .js/.css/.html files to modules and components to be a "full build system"? And you like to postpone it to manifest v3? I think we shouldn't wait that long. How about to switch to a "full build system" before manifest v3? This will also prevent you from writing a trivial script that will be replaced in the future.

eight04 avatar Dec 14 '21 17:12 eight04

  1. When our repo is checked out from git, there will be no dist, it'll be generated from src, just like in all normal projects.
  2. The build will support "watch" mode when only the changed files are automatically built. It's not what web-ext run does, but both features can work together.
  3. All our files will be normal ES modules with standard imports. It won't happen automagically. We'll need to do it manually.
  4. Half a year isn't that long to me, but I don't mind switching earlier as long as it accomplishes the above goals.

That said, for the time being I like the much less invasive option, which is: a) a simple bundle script for CodeMirror in build-vendor.js, b) a simple bundler for the main content scripts in zip.js, c) a modal UI for style settings so its js/css are loaded on demand, d) avoid adding new files to html in the meantime.

tophf avatar Dec 14 '21 19:12 tophf