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

Port to Parcel bundler #1137

Merged
merged 18 commits into from
Oct 3, 2019
Merged

Port to Parcel bundler #1137

merged 18 commits into from
Oct 3, 2019

Conversation

jtojnar
Copy link
Member

@jtojnar jtojnar commented Sep 25, 2019

Using asset bundler will allow us to switch to modern JavaScript, build user interface client-side more easily, and develop iteratively thanks to hot module replacement.

I chose Parcel over Webpack for simplicity of set up.

What remains

  • Determine what to do about skipWaiting, does the workbox-generated service worker have something similar? We no longer use Workbox library.
  • What is our caching strategy? Service worker should not be cached according to the comment workbox places into the file, but we do not use cache busting for it. Maybe that does not matter much, as browsers cannot keep workers more than 24 hours? We are cache busting /api/about requests and the HTML pages will not be cached regularly since they are returned from PHP (though service worker will still cache them, we will need to set them to network-first?).
  • Handle user.{js,css} better. Currently, we just throw <script> and <link rel="stylesheet"> into the DOM, hoping that something will be loaded. I would consider creating empty files in the repository to avoid 404s but there will need to for cache to be invalidated when they are changed as well. Or can we assume user that uses such customization is familiar with CTRL-F5?

What remains for future PRs

  • Workbox copies its files to the public/ directory, and then imports them using importScripts. Would it make sense to import it and let Parcel bundle it?
  • Check that babel is working correctly and that JavaScript files are properly shaken down (400 KB JS bundle is too big, https://github.com/gregtillbrook/parcel-plugin-bundle-visualiser will be helpful).
    • jQuery is full of dynamic hacks therefore hard to tree-shake and huge!
    • Stop Parcel from including huge node-libs-browser in hashpassword.jsbcryptjs supports using Node’s crypto library which makes Parcel think it should resolve it.
  • Move API to /api subdirectory. This will greatly simplify the rewrite rules.
  • Let web server serve the HTML files. What to do about cache invalidation? Do we need to update .htaccess/nginx conf…?
  • Refactor to use ES classes, Promises/A+, fetch…
  • Move the rest of the templates to client-side. We do not need EJS when we have JSX.
  • Make more things translatable.
  • Clean up the image references so that we do not have to copy those not recognized manually with parcel-plugin-static-files-copy. It will probably make more sense to switch directly to something like
    FontAwesome, though.
  • Add spinner for /api/about loading.
  • Do not use locales in the back-end.

@jtojnar jtojnar added this to the 2.19 milestone Sep 25, 2019
@jtojnar jtojnar mentioned this pull request Sep 25, 2019
@jtojnar
Copy link
Member Author

jtojnar commented Sep 25, 2019

I probably cannot make the bundling work with dynamically generated index.html, so I will need to add /config endpoint and make home.phtml static (which is something I wanted to do anyway).

We should make sure only data relevant for public will be leaked. And include user.js and user.css there.

It might be also cleaner if we move all APIs under /api and while at it, we could add /version endpoint.

This directory is not public facing, it contains assets used for building bundles.
It makes the repository slightly cleaner and makes things like linting easier.
* Do not run `npm install` in assets twice when running `dist` script.
* Install also PHP dependencies in post-install (re-use the `install-dependencies` target`).
* Use node executable from NPM via environment variable, instead of relying for it to be on PATH (which is not the case since NPM 4, see https://docs.npmjs.com/cli/run-script).
* Print filename of the generated archive in `dist` script.
Also follow semantic versioning so consumers are better informed about breakage.

https://semver.org/
This will allow clients to obtain version of Selfoss and API, as well as the the instance configuration. The configuration options are not part of the public API yet, though.

I put it under /api prefix, as we might move the rest of the API there in the future.

Increases API version to 2.20.0 – new feature
We will try to load fresh configuration from the server each time selfoss is opened and cache it in the localStorage indefinitely.

If configuration cannot be feched (e.g. in offline mode), the value from localStorage will be used.
@jtojnar
Copy link
Member Author

jtojnar commented Sep 26, 2019

Do Windows still support msapplication- meta tags. I cannot find any recent docs, only for IE. Not even for browserconfig.xml, which is still included in h5b.

I will remove it unless someone still wants it. cc @gleroi who introduced this in #609

@jtojnar
Copy link
Member Author

jtojnar commented Sep 27, 2019

With the latest push, selfoss appears to work when built with Parcel 🎉

Here is the commit message summarizing what has been done and what is to be:

Using asset bundler will allow us to switch to modern JavaScript, build user interface client-side more easily, and develop iteratively thanks to hot module replacement.

We chose Parcel over Webpack for simplicity of set up.

What changed

selfoss no longer supports run-time minification of assets. Instead, developer will need to run npm run build to bundle all the dependencies and our files and copy them to the public/ directory. This means the directory is no longer needed to be writeable at run time (useful for read only locations). Alternately, developer can run npm run dev command, which will start a program that will watch selfoss files, rebuild them in the background when changed, and then, refresh the opened page. For common users, there are prebuilt packages available, or they can use user.{css,js} for customization, as before.

The routes GET / (without AJAX), GET /password and GET /opml were turned into primitive servers of static HTML files and all the magic is happening client-side. OPML is uploaded completely using XHR and even the password hash is generated client-side.

We started using some EcmaScript 6 features like imports, which Parcel will automatically resolve.

Languages are imported statically as well, to avoid having to copy them using parcel-plugin-static-files-copy. Unfortunately, it requires us to declare them explicitly for Parcel to be able to resolve them. And just like with the selfoss.config function replacing the data attribute lookup, I added selfoss.lang that reads the imported locale files directly.

We are no longer setting <base> in the HTML files but that was not actually necessary, as we do not use subdirectories in our front-end routes. When we start using history API, it will not be necessary either, as long as we know the routes. The base URI is therefore only necessary for cookies.

We are using parcel-plugin-sw-cache, which uses workbox to generate ServiceWorker boilerplate. Actually, we no longer write any ServiceWorker code.

What remains

  • Determine what to do about skipWaiting, does the workbox-generated service worker have something similar?
  • What is our caching strategy? Service worker should not be cached according to the comment workbox places into the file, but we do not use cache busting for it. Maybe that does not matter much, as browsers cannot keep workers more than 24 hours? We are cache busting /api/about requests and the HTML pages will not be cached regularly since they are returned from PHP (though service worker will still cache them, we will need to set them to network-first?).
  • Handle user.{js,css} better. Currently, we just throw <script> and <link rel="stylesheet"> into the DOM, hoping that something will be loaded. I would consider creating empty files in the repository to avoid 404s but there will need to for cache to be invalidated when they are changed as well. Or can we assume user that uses such customization is familiar with CTRL-F5?
  • Check that babel is working correctly and that JavaScript files are properly shaken down (400 KB JS bundle is too big, https://github.com/gregtillbrook/parcel-plugin-bundle-visualiser will be helpful).

What remains for future PRs

  • Workbox copies its files to the public/ directory, and then imports them using importScripts. Would it make sense to import it and let Parcel bundle it? How to pre-process the service worker with Parcel? mischnic/parcel-plugin-sw-cache#22
  • Refactor to use ES classes, Promises/A+, fetch…
  • Move the rest of the templates to client-side. (Switch to client-side templating #928) We do not need EJS when we have JSX.
  • Make more things translatable.
  • Clean up the image references so that we do not have to copy those not recognized manually with parcel-plugin-static-files-copy. It will probably make more sense to switch directly to something like FontAwesome, though. (Change icons to vector #1013)
  • Add spinner for /api/about loading.
  • Do not use locales in the back-end.
  • Use flow annotations

@gleroi
Copy link
Contributor

gleroi commented Sep 27, 2019

msapplication- was introduced mainly for windows phone. With the death of the platform i think it can be safely removed.
The tag may be still supported by windows 10, but microsoft recommends to use a standard web.manifest

The use of template strings make this commit unusable in older browsers until we start transpiling with Parcel.
It is not so bad: https://caniuse.com/#feat=template-literals
So that we can make the templates completely static.

Also fix some invalid HTML and meta parity.
These use their own salt so it will make them easier to generate on the client. We still support the old password scheme with separate salt so users did not have to rehash their passwords.
@jtojnar jtojnar force-pushed the parcel branch 2 times, most recently from 806afac to 1688e8e Compare September 27, 2019 11:26
assets/js/selfoss-base.js Outdated Show resolved Hide resolved
$this->view->hash = hash('sha512', \F3::get('salt') . $_POST['password']);
}
echo $this->view->render('templates/hashpassword.phtml');
echo file_get_contents(BASEDIR . '/public/hashpassword.html');
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe remove this function and serve the static file from the webserver using rewriting?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I started with / and I initially wanted to do this but the fact that we use / for both the page and the API endpoint prevented me from doing this. Then I did not re-evaluate it for the other pages. But since GET / is not actually a part of public API, GET /items is, we probably could get away with moving it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One benefit of sending them through PHP is that we do not have to disable caching in the web server. To not delay this any further, I decided not to keep the current behaviour for now.

@niol
Copy link
Collaborator

niol commented Oct 2, 2019

This is really interesting for a number of reasons.
I would have preferred to go with webpack which seems more popular and that would have forced me to finally learn it.
For the skipWaiting issue, it seems workbox build support a skipWaiting boolean argument that defaults to true. Sending {type: 'SKIP_WAITING'} in selfoss-base.js should work.

@jtojnar
Copy link
Member Author

jtojnar commented Oct 2, 2019

Yeah, I think we might need to go back to Webpack after all. Parcel is currently being completely rewritten to be more configurable and the plug-ins for the old version are not very flexible.

WebPack has a plug-in for Workbox straight from the horse’s mouth.

I really liked how Parcel has built-in support for hot-module reloading, overlay error reporting and I also felt that the HTML/CSS handling is much better in Parcel since Webpack currently only supports JavaScript and getting a CSS file is a hideous hack, but mischnic/parcel-plugin-sw-cache#22 is a deal breaker.

@jtojnar
Copy link
Member Author

jtojnar commented Oct 2, 2019

By the way this is the service worker which https://github.com/mischnic/parcel-plugin-sw-cache generates. skipWaiting is called in top-level:

/**
 * Welcome to your Workbox-powered service worker!
 *
 * You'll need to register this file in your web app and you should
 * disable HTTP caching for this file too.
 * See https://goo.gl/nhQhGp
 *
 * The rest of the code is auto-generated. Please don't update this file
 * directly; instead, make changes to your Workbox build configuration
 * and re-run your build process.
 * See https://goo.gl/2aRDsh
 */

importScripts("workbox-v4.3.1/workbox-sw.js");
workbox.setConfig({modulePathPrefix: "workbox-v4.3.1"});

workbox.core.skipWaiting();

workbox.core.clientsClaim();

/**
 * The workboxSW.precacheAndRoute() method efficiently caches and responds to
 * requests for URLs in the manifest.
 * See https://goo.gl/S9QRab
 */
self.__precacheManifest = [
  {
    "url": "arrow-down.fe617440.png",
    "revision": "d643bd115b8856864d69dcdd1cd64d21"
  },
  …
  {
    "url": "/",
    "revision": "0d728a7eb5884c34a6b36f32cfe8072c"
  }
].concat(self.__precacheManifest || []);
workbox.precaching.precacheAndRoute(self.__precacheManifest, {});

workbox.routing.registerNavigationRoute(workbox.precaching.getCacheKeyForURL(".//index.html"));

@jtojnar
Copy link
Member Author

jtojnar commented Oct 2, 2019

The manifest injection seems to work at least.

@jtojnar jtojnar mentioned this pull request Oct 2, 2019
Just like with the `selfoss.config` replacing the `data` attribute lookup, we should use the existing `selfoss.ui._` function. This will make changing the translation method easier in the future.
Setting `<base>` in the HTML files was not actually necessary, since we do not use subdirectories in our front-end routes. When we start using history API, it will not be necessary either, as long as we know the routes. The base URI is therefore only necessary for cookies and RSS feed.
@jtojnar jtojnar marked this pull request as ready for review October 3, 2019 00:57
Using asset bundler will allow us to switch to modern JavaScript, build user interface client-side more easily, and develop iteratively thanks to hot module replacement.

We chose Parcel over Webpack for simplicity of set up.

With this, selfoss no longer supports run-time minification of assets. Instead, developer will need to run `npm run build` to bundle all the dependencies and our files and copy them to the `public/` directory. This means the directory is no longer needed to be writeable at run time (useful for read only locations). Alternately, developer can run `npm run dev` command, which will start a program that will watch selfoss files, rebuild them in the background when changed, and then, refresh the opened page. For common users, there are prebuilt packages available, or they can use user.{css,js} for customization, as before.
The routes `GET /` (without AJAX), `GET /password` and `GET /opml` were turned into primitive servers of static HTML files and all the magic is happening client-side. OPML is uploaded completely using XHR and even the password hash is generated client-side.

We started using some EcmaScript 6 features like imports, which Parcel will automatically resolve.

Languages are imported statically as well, to avoid having to copy them using `parcel-plugin-static-files-copy`. Unfortunately, it requires us to declare them explicitly for Parcel to be able to resolve them.

We are using `parcel-plugin-sw-cache`, which uses `workbox` to generate ServiceWorker boilerplate. We do not use the Workbox library, though, as it is not possible to both load it locally (not from CDN) and customize the service worker – we just rely on it to get the list of files into the worker.
Since these are supposed to be editable by user any time and we do not want to require users to install development environment, we will need fetch these files manually and handle cache invalidation ourselves. As a key, we will use their mtimes, which we will pass through the configuration key of `/api/about` endpoint. This is re-fetched whenever selfoss is opened, so we can notice when the files are modified, and we will add them to browser's cache storage. Service worker will ignore those cache keys.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants