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

Loading subapps from a different repo #1700

Merged
merged 13 commits into from
Jul 16, 2020

Conversation

divyakarippath
Copy link
Contributor

@divyakarippath divyakarippath commented Jul 14, 2020

Directory structure of individual subapps:

my-sub-app
├── README.md
├── package.json
├── src
........├── index.js
........├── subapp.js
........├── server.js
........├── reducers.js

Each subapp should be exporting the manifest and sub-app options as below

const subAppOptions = {
  serverSideRendering: false
};

const manifest = {
  type: "app",
  name: "Demo1",
  entry: require.resolve("./subapp"),
  serverEntry: require.resolve("./server"),
  reducers: require.resolve("./reducers")
};

module.exports = { subAppOptions, manifest `};

The users must provide absolute paths for entry, serverEntry and reducers
Publishing : The code must be transpiled to node executable format before publishing the package.

Main app:

Create a file named subapps-modules.js in src and export the manifests for all the subapp modules

const demo1 = require("demo1");
const demo2 = require("demo2");

module.exports = {
  demo1: demo1.manifest,
  demo2: demo2.manifest
};

Update the routes file to load the subapp module.

export default {
"/home": {
    pageTitle: "Welcome to home",
    subApps: ["demo1", "demo2"]
  }
}

Main app can also customize the options for subapp as below.

"/home": {
    pageTitle: "Welcome to home",
    subApps: [["demo1", subAppOptions], ["demo2", subAppOptions]]
  }
}

x = x[0];
}
// absolute: use as path
// else: assume dir under srcDir
// TBD: handle it being a module
if (x.indexOf("/") === -1) {
Copy link
Member

Choose a reason for hiding this comment

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

what's this trying to do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is to handle the scenario where subapp name is just the module name (when loading from a separate repo)

"/demo1": {
    pageTitle: "Welcome to Demo1",
    subApps: [["demo1", demo1.subAppOptions]],
  },

For the sub-apps from a separate repo, its path is set as Path.dirname(require.resolve(`${modName}/package.json`)) in subAppsByPath. Same logic is used here to retrive the details from the map.

Another option I was considering it is to set module:true in subappOptions and use that flag instead of checking for / in url. But that would be one additional thing to be set by the main app.

In existing code, if path is not absolute, we assume it's relative to app src dir.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the code to remove module:true flag

const entries = _.pick(manifest, ["entry", "serverEntry", "reducers"]);
Object.keys(entries).forEach(x => {
if (!Path.isAbsolute(entries[x])) {
throw new Error(`Error: manifest ${x} requires absolute path`);
Copy link
Member

Choose a reason for hiding this comment

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

error message is not very clear

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the error message in latest commit

if (subAppPath) {
const { manifest, subAppOptions } = require(x);
x = manifest ? Path.dirname(subAppPath) : x;
options = options || subAppOptions;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added this to support passing subapp options from main app also. This will be useful in use-cases where main app wants to customize the options for individual subapps.

@@ -62,7 +62,7 @@
"@xarc/app-dev": "^5.3.4",
"electrode-archetype-opt-critical-css": "^1.0.3",
"electrode-archetype-opt-eslint": "^1.0.3",
"electrode-archetype-opt-jest": "^25.0.0",
"electrode-archetype-opt-jest": "^26.0.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed to match the version with latest publish

@@ -55,6 +55,7 @@
"lodash": "^4.17.10",
"milligram": "^1.3.0",
"react-intl": "^3.11.0",
"intl-format-cache": "4.2.46",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To fix the build issue

@jchip jchip merged commit c3e45e0 into electrode-io:master Jul 16, 2020
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.

2 participants