wp-pwa / wp-pwa

This project was renamed and moved to https://github.com/frontity.
https://frontity.org
Apache License 2.0
36 stars 7 forks source link

[Design] Figure out the new project structure #93

Open luisherranz opened 6 years ago

luisherranz commented 6 years ago

A Frontity project should...

The current list of params passed to the server are:

Child of #92

luisherranz commented 6 years ago

Maybe the file-settings package should be the default one, but an alternative one could be passed via params/env variable, and that could be configurable via npm scripts for the user if it's useful in her local environment like npm run start:graphql. Useful in production environments where you always want it to work with your db (i.e. SETTINGS_PACKAGE="@frontity/graphql-settings").

luisherranz commented 6 years ago

Packages (both normal and settings) could be found in both /packages or /node_modules. Naming/folders would have to respect npm organizations (if orgs are used). For example, to import @frontity/wp-org-connection, first it searches in /packages. If it's not there, it searches in /node_modules. If you clone @frontity/wp-org-connection, the folder should be wp-org-connection and it should be inside /packages/@frontity/.

luisherranz commented 6 years ago

Maybe amp mode should be triggered when the url contains a final /amp folder, a ?amp query or a amp. subdomain. It's not perfect, but it may work. It could be changed somehow too, but where?

luisherranz commented 6 years ago

Maybe a frontity.config.js file could be optional. It could have low stuff like the function which detects amp, or the settings package:

{
  settingsPackage: '@frontity/graphql-settings',
  isAmp: ({ url }) => /(^https:\/\/amp\.|\/amp$|[?&]amp)/i.test(url),
}
luisherranz commented 6 years ago

Choosing which setting to use with each site only knowing the url could be solved by a urls field in each site configuration:

export default ({ url }) => ({
    site: 'blog-frontity',
    urls: [
      'default',
      'blog.frontity.com',
      'amp.frontity.com',
      'localhost:3000',
    ],

The problem here is where you have two sites in the same domain, ie: diariomotor.com and diariomotor.com/competicion. In this case, the algorithm should look first for the site with the highest number of existing folders. For example, for diariomotor.com/competicion/category/formula1 it should try:

luisherranz commented 6 years ago

Choosing which namespaces to use from each package could be solved using the active field of the package:

{
  ...
  package: '@frontity/saturn-theme',
    active: ['theme', 'h2r', 'router'],
    settings: { ...
luisherranz commented 6 years ago

The structure of a frontity project could be:

── my-frontity-project
    ├── frontity (cloned from github.com/frontity/frontity/...)
    ├── frontity.config.js
    ├── node_modules
    ├── package.json
    ├── packages
    │   └── @frontity
    │       ├── saturn-theme
    │       └── wp-org-connection
    └── settings
        ├── blog-frontity.js
        └── index.js

If frontity is cloned inside the /frontity folder, the npm script would be simply:

node frontity/index.js

and if frontity is a npm package installed with npm i frontity, the npm script would be:

frontity

We have to add a script in node_modules/.bin/ and figure out an API to not only start, but build+ andserver` as well. Maybe that could be params:

frontity (like --start now)
frontity --build
frontity --server
luisherranz commented 6 years ago

To add stuff to Webpack, we could use a similar approach to nextjs, with a webpack.config.js in the root folder of the project. Same for babel with babel.config.js.

These files won't override the default configuration. Instead, that default configuration is passed to a function so complex manipulations can be done.

Frontity packages (themes, extensions, etc) shouldn't be transpiled. Webpack should be in charge of that transpilation even if they are inside node_modules.

luisherranz commented 6 years ago

dynamic and static urls can be set in execution but they need to be known in SSR (with webpack's public_path setting) so maybe they can go in settings somehow.

luisherranz commented 6 years ago

We can ditch siteId in favor of site. Now it should be optional, so you can still test stuff like: https://blog.frontity.com/?server=https://localhost:3000&site=blog-frontity-dev

luisherranz commented 6 years ago

Remaining questions are:

luisherranz commented 6 years ago

create-react-app v2 beta compiles node_modules using "new JavaScript features". New features could mean ES6-7 only or maybe a usage of "jsnext:main": "index.js". We should take a look at what they're doing.

https://github.com/facebook/create-react-app/pull/3776

luisherranz commented 6 years ago

To avoid using npm link but still be able to run a local package when the settings return a remote package (settings: @frontity/saturn-theme, local: ./packages/saturn-theme) maybe we can add an alias setting in frontity.config.js like this:

alias: {
  '@frontity/saturn-theme': './packages/saturn-theme',
   ...
}
luisherranz commented 6 years ago

Maybe active could be both an array of namespaces like active: ['theme', 'h2r', 'router'], or just true which means include everything. The problem is that this would make more difficult the dashboard logic, but easier to just activate packages in the file settings.

luisherranz commented 6 years ago

Last idea: frontity.config.js could be present inside packages, even if they are in node_modules. That way, they can add their own ES6 modules to webpack's externals list to get transpiled in node, which doesn't support ES6 modules yet.

luisherranz commented 6 years ago

frontity.config.js would have different properties, like for example webpack or babel. These are functions to process the original webpack and babel files.

Loading order would be:

luisherranz commented 6 years ago

We should centralize all configuration inside frontity.config.js files so there's only one place to look and one file to learn.

luisherranz commented 6 years ago

We could use yalc to develop local packages, instead of npm/yarn link because of the symlink problems. It copies the npm-code to the project .yarn folder and creates a symlink but for that local folder with yalc link package-name. This way, node_modules are shared.

The only technical problem we need to solve is watching that code, but it looks like it can be solved using nodemon: https://github.com/whitecolor/yalc/issues/22 It could be a bit cumbersome to run nodemon -x 'yarn push' for each package. Maybe that could be included in npm run start -- -d. We could inspect yalc.lock to know the local packages. Webpack should watch for files inside the .yarn folder.

Once we are "in the cloud", an npm install should retrieve the final npm package.

This way we can keep only one package name in the settings.


[UPDATE]: After all this investigation, it looks like having the code inside /packages and creating symlinks from node-modules to /packages may just work because that's what yalc link does, but going through ~/.yalc.

We wouldn't need to watch anything or change package.json.

Not sure how to create Windows symlinks though, but it has to be possible.

Maybe we can create those symlinks automatically, or at least give a tool to create them:

frontity-project/node_modules/frontity => frontity-project/frontity
frontity-project/node_modules/my-package => frontity-project/packages/my-package
luisherranz commented 6 years ago

ESModules are not currently supported in node, so in the webpack/server.config.js we need to make sure those are included in the final bundle (m.js) and transpiled appropiately. We need a list of frontity packages to be able to exclude them from externals.

luisherranz commented 6 years ago

The query server can be used to load an alternative settings/server in the domain, so it should be restricted with some kind of auth.

luisherranz commented 6 years ago

We can have a url per site. For example, mysite.frontity.io. host can be sent to the settings package, and it will be unique. It works for injection, php theme and with a CDN, with direct domain too. Even for AMP as the host will still be mysite.frontity.io.

We would need to detect AMP somehow.

It also opens the door to use serverless and avoid the monolithic repo + easier to manage custom code from customers (as it is isolated in their serverless docker).

luisherranz commented 6 years ago

We need to be able to hook into the url discovery system. For example, many blogs use pages for the home or some categories, whereas we'd like to retrieve actual categories from the WP API instead of the page.

It'd be something like extending the current forceFrontPage functionality but hooked into the discovery system.

[UPDATE]: Maybe this can be solved with the idea that connection should have a collection of urls that match a specific endpoint+params configuration.

luisherranz commented 6 years ago

We need to be able to serve Frontity PWA assets from within WP. Yep, we'll lose the ability to do SSR.

It'd be easier if the setup doesn't involve programming, maybe using a shortcode in a page or something like that. The shortcode could be [frontity].

Maybe it also needs a js code injected with wp_head to remove <body> and leave only a <div class="root"> for React. It could:

Frontity and the WP plugin will share the same folder. .build can't be in the .gitignore.

<head> could remaing untoched.

ReactDOM should do render instead of hydrate.

Once the PWA has loaded, the user must be able to navigate other pages using the PWA.

The shortcode could include options, like [frontity mode="mobile,tablet"].

The WP plugin must include the Frontity assets, so they need to be built and included in the plugin files.

Maybe what Frontity needs to do, is to make it really simple to create WP plugins like this. Or WP themes.

Settings can be populated in WP and injected via the global variable initialState. MST can get them from there, as it usually does with SSR.

luisherranz commented 6 years ago

[UPDATE]: I found a better solution explained in the comment below.

luisherranz commented 6 years ago

Or even better, we can generate matches without any restriction, then use them for settings:

[
  {
    "site": "my-blog",
    "matches": {
      "amp_localhost": [
        "localhost:3000.+/amp/?$",
        "localhost:3000.+[?&}amp=true"
      ],
      "main_localhost": "localhost:3000",
      "amp": [
        "amp.my-blog.com",
        "my-blog.com.+/amp/?$",
        "my-blog.com.+[?&}amp=true"
      ],
      "main": "my-blog.com"
    },
    "settings": [
      {
        "matches": ["amp_localhost", "main_localhost"],
        "namespaces": "frontity",
        "data": {
          "publicPath": "https://localhost:3000"
        }
      },
      {
        "matches": ["amp", "main"],
        "namespaces": "frontity",
        "data": {
          "publicPath": "https://v1.frontity.io"
        }
      },
      {
        "matches": ["main", "main_localhost"],
        "namespaces": "frontity",
        "data": {
          "url": "https://my-blog.com"
        }
      },
      {
        "matches": ["amp", "amp_localhost"],
        "namespaces": "frontity",
        "data": {
          "url": "https://amp.my-blog.com"
        }
      }
    ],
    "packages": {
      "@frontity/saturn-theme": {
        "settings": [
          {
            "matches": "*",
            "namespaces": "theme",
            "data": {
              "color": "#AAA"
            }
          },
          {
            "matches": "*",
            "namespaces": "h2r",
            "data": {
              "removeWpEmbeds": true
            }
          }
        ]
      },
      "@frontity/analytics": {
        "settings": [
          {
            "matches": "main",
            "namespaces": "analytics",
            "data": {
              "analytics": "UA-001"
            }
          },
          {
            "matches": "amp",
            "namespaces": "analytics",
            "data": {
              "analytics": "UA-002"
            }
          }
        ]
      },
      "@frontity/wp-org-connection": {
        "settings": [
          {
            "matches": "*",
            "namespaces": "connection",
            "data": {
              "endpoint": "https://my-blog.com/wp-json/",
              "perPage": 13
            }
          }
        ]
      }
    }
  }
]

If settings is an array, it accepts matches, namespaces and data. Default matches is "*" and default namespace is all namespaces from the package.

The super simple version would be something like:

  {
    "site": "my-blog",
    "settings": {
      "url": "https://my-blog.com"
    },
    "packages": {
      "@frontity/saturn-theme": {
        "settings": {
          "color": "#AAA"
        }
      },
      "@frontity/wp-org-connection": {
        "settings": {
          "endpoint": "https://my-blog.com/wp-json/",
          "perPage": 13
        }
      }
    }
  }

Which is the equivalent to:

[
  {
    "site": "my-blog",
    "matches": {
      "default": "*"
    },
    "settings": [
      {
        "matches": "default",
        "namespaces": "frontity",
        "data": {
          "url": "https://my-blog.com"
        }
      }
    ],
    "packages": {
      "@frontity/saturn-theme": {
        "settings": [
          {
            "matches": "default",
            "namespaces": "theme",
            "data": {
              "color": "#AAA"
            }
          }
        ]
      },
      "@frontity/wp-org-connection": {
        "settings": [
          {
            "matches": "default",
            "namespaces": "connection",
            "data": {
              "endpoint": "https://my-blog.com/wp-json/",
              "perPage": 13
            }
          }
        ]
      }
    }
  }
]

Namespaces are extracted from package namespaces.

If we don't want namespaces to be defined by the user in settings.json (so we can work without package.json file, they should be defined somehow by the package creator.

luisherranz commented 6 years ago

This way amp is not a even an internal concept anymore, it's just one custom match.

Settings, inside Frontity, would look like:

store.settings.frontity.url; // <= either https://my-blog.com or https://amp.my-blog.com
store.settings.theme.color; // <= #AAA
store.settings.connection.endpoint; // <= https://my-blog.com/wp-json/
store.settings.h2r.removeWpEmbed; // <= true
store.settings.analytics.url; // <= either UA-001 or UA-002
store.build.match; // <= either main, amp, main_localhost or amp_localhost
store.build.site; // <= my-blog
store.settings.frontity.publicPath; // <= either https://localhost:3000 or https://v1.frontity.io

Not sure yet if store.build is necessary. Maybe it can be merged with store.settings.frontity.

luisherranz commented 6 years ago

Themes or extensions that support things like amp are going to need something passed to know they are using amp. Or maybe they can rely on things like

const isAmp = () => /amp/.test(store.build.match);

So if the match contains the word "amp", they are in amp, if they don't (maybe main or default) then they are not.

That way we can use both amp and amp_localhost.

In the super simple case, settings would look like:

  {
    "site": "my-blog",
    "matches": {
      "amp": "/amp/$",
      "main": "*"
    },
    "settings": {
      "url": "https://my-blog.com"
    },
    "packages": {
      "@frontity/saturn-theme": {
        "settings": {
          "color": "#AAA"
        }
      },
      "@frontity/wp-org-connection": {
        "settings": {
          "endpoint": "https://my-blog.com/wp-json/",
          "perPage": 13
        }
      }
    }
  }

So not that bad.

luisherranz commented 6 years ago

store.build now contains:

So store.build would look like this:

store.build.site  // <= my-blog
store.build.match // <= either amp or main
store.build.device // <= tablet, mobile, desktop
store.build.rendering // <= either csr or ssr
store.build.packages // <= array of packages
store.build.entryUrl // <= array of packages

After the merge with store.settings.frontity, it'd look like this:

store.settings.frontity.site // <= my-blog
store.settings.frontity.match // <= either amp or main
store.settings.frontity.device // <= tablet, mobile, desktop
store.settings.frontity.rendering // <= either csr or ssr
store.settings.frontity.packages // <= array of packages
store.settings.frontity.entryUrl // <= array of packages

It doesn't make much sense. Settings are things declared by the user and saved in the db.

luisherranz commented 6 years ago

Every pattern is a regepx pattern by default. The string "*" can be translated to ".*" in code.

luisherranz commented 6 years ago

Even matches could be used as an array:

{
  "matches": [
    {
      "name": "amp",
      "patterns": "/amp/$",
      "type": "regexp",
    },
    {
      "name": "main",
      "patterns": "**/*",
      "type": "globs",
    }
  ]
}
luisherranz commented 6 years ago

We can also use matches for entire packages.

Image we have two versions of the same theme, one for mobile and the other for desktop:

  {
    "site": "my-blog",
    "matches": {
      "mobile": "m.my-blog.com",
      "main": "my-blog.com"
    },
    "settings": {
      "url": "https://my-blog.com"
    },
    "packages": {
      "@my-org/mobile-theme": {
        "matches": "mobile",
        "settings": {
          "color": "#AAA"
        }
      },
      "@my-org/main-theme": {
        "matches": "main",
        "settings": {
          "color": "#BBB"
        }
      },
      "@frontity/wp-org-connection": {
        "settings": {
          "endpoint": "https://my-blog.com/wp-json/",
          "perPage": 13
        }
      }
    }
  }

We can add other types of matches:

  {
    "site": "my-blog",
    "matches": [
      {
        "name": "mobile",
        "pattern": "(Android .* Mobile|ip(ad|hone)",
        "type": "userAgent"
      },
      {
        "name": "main",
        "pattern": "*",
        "type": "userAgent"
      }
    ],
    "settings": {
      "url": "https://my-blog.com"
    },
    "packages": {
      "@my-org/mobile-theme": {
        "matches": "mobile",
        "settings": {
          "color": "#AAA"
        }
      },
      "@my-org/main-theme": {
        "matches": "main",
        "settings": {
          "color": "#BBB"
        }
      },
      "@frontity/wp-org-connection": {
        "settings": {
          "endpoint": "https://my-blog.com/wp-json/",
          "perPage": 13
        }
      }
    }
  }

We still have to think a way to match both the url and the userAgent.

Maybe something like:

"matches": [
      {
        "name": "mobile",
        "patterns": [
          {
            "pattern": "(Android .* Mobile|ip(ad|hone)",
            "type": "userAgent"
          },
          {
            "pattern": "my-blog.com",
            "type": "regexp"
          }
        ]
      },
      {
        "name": "main",
        "pattern": "my-blog.com"
      }
    ],
luisherranz commented 6 years ago

If we force a site with ?site=my-blog we need to define which matches triggers.

Maybe a "default" match should be included:

  {
    "site": "my-blog",
    "matches": {
      "amp": "^my-blog.+/amp/$",
      "default": "^my-blog"
    },
    "settings": {
      "url": "https://my-blog.com"
    },
    "packages": {
      "@frontity/saturn-theme": {
        "settings": {
          "color": "#AAA"
        }
      },
      "@frontity/wp-org-connection": {
        "settings": {
          "endpoint": "https://my-blog.com/wp-json/",
          "perPage": 13
        }
      }
    }
  }

So in case the url doesn't match, default is triggered.

And... you can overwrite it as well: ?site=my-blog&match=amp

luisherranz commented 6 years ago

It may be useful to have access to other matched settings.

For example:

store.settings.frontity.url // <= https://my-blog.com
store.settings.frontity.matches.default.url // <= same, https://my-blog.com
store.settings.frontity.matches.amp.url // <= https://amp.my-blog.com
luisherranz commented 6 years ago

Wondering if store.settings.frontity should be store.settings.build and include site settings:

store.settings.build.site // <= my-blog
store.settings.build.match // <= either amp or main
store.settings.build.device // <= tablet, mobile, desktop
store.settings.build.rendering // <= either csr or ssr
store.settings.build.packages // <= array of packages
store.settings.build.entryUrl // <= https://my-blog.com/my-post
store.settings.build.url // <= https://my-blog.com
store.settings.build.publicPath // <= https://v1.frontity.io
luisherranz commented 6 years ago

We can save real settings in store.rawSettings and use a view to return them:

const Store = types.model("Store", {
  rawSettings: types.frozen(),
}).views(self => ({
  get settings() {
    return self.rawSettings;
  }
});
luisherranz commented 6 years ago

Or maybe with hooks we don't need to create that many views:

const Component = () => {
  const isSsr = useStore(store => store.settings.build.rendering) === 'ssr';
  return (
    <div>{isSsr ? 'SSR!' : 'CSR!'}</div>
  )
};

And they can be reusable:

const useSsrState = () => useStore(store => store.settings.build.rendering) === 'ssr';

const Component = () => {
  const isSsr = useSsrState();
  return (
    <div>{isSsr ? 'SSR!' : 'CSR!'}</div>
  )
};