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

Fixed some follow ups after restructuring and issues around generator-theia #310

Merged
merged 10 commits into from
Jul 20, 2017
1 change: 1 addition & 0 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ before_script:
before_install:
- "npm install -g npm@^4"
- "google-chrome-stable --headless --disable-gpu --remote-debugging-port=9222 http://localhost &"
bundler_args: --retry 0
Copy link

Choose a reason for hiding this comment

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

Is this travis specific or it's passed to lerna or something ?
I can't find options on lerna or npm and travis ci references bundler_args with the ruby bundler

Copy link
Member Author

Choose a reason for hiding this comment

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

It is travis specific, depending on provided language travis knows how to build the project, like run npm install first and then npm test for node projects. It also has some defaults for such projects, e.g. for node to try to run npm install 3 times if the first fails, since npmjs sometimes is not available.

Copy link

Choose a reason for hiding this comment

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

OK thanks!

notifications:
webhooks:
urls:
Expand Down
18 changes: 18 additions & 0 deletions .vscode/launch.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,24 @@
// For more information, visit: https://go.microsoft.com/fwlink/?linkid=830387
"version": "0.2.0",
"configurations": [
{
"type": "node",
"request": "launch",
"name": "Launch Electron Backend",
"runtimeExecutable": "${workspaceRoot}/node_modules/.bin/electron",
"program": "${workspaceRoot}/examples/electron/src-gen/frontend/electron-main.js",
"protocol": "legacy",
"args": [
"--loglevel=debug"
],
"sourceMaps": true,
"outFiles": [
"${workspaceRoot}/examples/electron/src-gen/frontend/electron-main.js",
"${workspaceRoot}/examples/electron/src-gen/backend/main.js",
"${workspaceRoot}/examples/browser/lib/**/*.js",
"${workspaceRoot}/packages/*/lib/**/*.js"
]
},
{
"type": "node",
"request": "launch",
Expand Down
25 changes: 19 additions & 6 deletions doc/Developing.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,16 +41,28 @@ This command does a few things:

## Run the browser-based example application

We can start the application with:
We can start the application from the examples/browser directory with:

npm start

This command starts the backend application, a small web server and a browser
tab with the frontend.

If you rebuild native node packages for electron then
rollback these changes before starting the browser example
by running from the root directory:

npm run rebuild:web

## Run the electron-based example application

It can also be started with:
From the root directory run:

npm run rebuild:electron

This command rebuilds native node packages against the version of node used by electron.

It can also be started from the examples/electron directory with:

npm start

Expand Down Expand Up @@ -116,16 +128,17 @@ To build and run the browser example:

git clone https://github.com/theia-ide/theia \
&& cd theia \
&& npm install --unsafe-perm\
&& cd ../../examples/browser \
&& npm install --unsafe-perm \
Copy link

Choose a reason for hiding this comment

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

BTW we discussed this a bit and --unsafe-perm looks really weird.
I Suggest we move it out of the TL;DR and add it to the troubleshooting section
but that can be in another PR

Copy link
Member Author

@akosyakov akosyakov Jul 19, 2017

Choose a reason for hiding this comment

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

Could you open an issue for it? or PR :)

Copy link

Choose a reason for hiding this comment

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

Issue #311

&& cd examples/browser \
&& npm run start

To build and run the electron example:

git clone https://github.com/theia-ide/theia \
&& cd theia \
&& npm install --unsafe-perm\
&& cd ../../examples/electron \
&& npm install --unsafe-perm \
&& npm run rebuild:electron \
&& cd examples/electron \
&& npm run start

## Code coverage
Expand Down
25 changes: 10 additions & 15 deletions doc/Internals.md
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ import { ILoggerServer, ILoggerClient } from '../../application/common/logger-pr

export const loggerServerModule = new ContainerModule(bind => {
bind(ConnectionHandler).toDynamicValue(ctx =>
new JsonRpcConnectionHandler<ILoggerClient>("/logger", client => {
new JsonRpcConnectionHandler<ILoggerClient>("/services/logger", client => {
const loggerServer = ctx.container.get<ILoggerServer>(ILoggerServer);
loggerServer.setClient(client);
return loggerServer;
Expand Down Expand Up @@ -124,7 +124,7 @@ To dig more into ContributionProvider see this [section](#contribution-providers
So now:

``` typescript
new JsonRpcConnectionHandler<ILoggerClient>("/logger", client => {
new JsonRpcConnectionHandler<ILoggerClient>("/services/logger", client => {
```

This does a few things if we look at this class implementation:
Expand Down Expand Up @@ -198,20 +198,15 @@ And it returns the loggerServer as the object that will be exposed over JSON-RPC

This connects the factory to the connection.

So now all is set for backend/frontend communication.

The only point left is that if you're using the webpack dev server which
you probably are you need to add something like this:
The endpoints with `services/*` path are served by the webpack dev server, see `webpack.config.js`:

``` javascript
'/logger/*': {
target: 'ws://localhost:3000',
ws: true
},
'/services/*': {
target: 'ws://localhost:3000',
ws: true
},
```

To webpack.config.js so that the requests are proxied to the backend properly.

## Connecting to a service

So now that we have a backend service let's see how to connect to it from
Expand All @@ -234,7 +229,7 @@ export const loggerFrontendModule = new ContainerModule(bind => {
bind(ILoggerServer).toDynamicValue(ctx => {
const loggerWatcher = ctx.container.get(LoggerWatcher);
const connection = ctx.container.get(WebSocketConnectionProvider);
return connection.createProxy<ILoggerServer>("/logger", loggerWatcher.getLoggerClient());
return connection.createProxy<ILoggerServer>("/services/logger", loggerWatcher.getLoggerClient());
}).inSingletonScope();
});
```
Expand All @@ -245,7 +240,7 @@ The important bit here are those lines:
bind(ILoggerServer).toDynamicValue(ctx => {
const loggerWatcher = ctx.container.get(LoggerWatcher);
const connection = ctx.container.get(WebSocketConnectionProvider);
return connection.createProxy<ILoggerServer>("/logger", loggerWatcher.getLoggerClient());
return connection.createProxy<ILoggerServer>("/services/logger", loggerWatcher.getLoggerClient());
}).inSingletonScope();

```
Expand All @@ -269,7 +264,7 @@ See more information about how events works in theia [here](events).
Here we're getting the websocket connection, this will be used to create a proxy from.

``` typescript
return connection.createProxy<ILoggerServer>("/logger", loggerWatcher.getLoggerClient());
return connection.createProxy<ILoggerServer>("/services/logger", loggerWatcher.getLoggerClient());
```

So here at the last line we're binding the ILoggerServer interface to a
Expand Down
12 changes: 2 additions & 10 deletions examples/browser/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ const CopyWebpackPlugin = require('copy-webpack-plugin');
const CircularDependencyPlugin = require('circular-dependency-plugin');

const outputPath = path.resolve(__dirname, 'lib');
const emptyPath = path.resolve(__dirname, 'webpack_empty.js');

const monacoEditorPath = '../../node_modules/monaco-editor-core/min/vs';
const monacoLanguagesPath = '../../node_modules/monaco-languages/release';
Expand Down Expand Up @@ -64,18 +63,13 @@ module.exports = {
resolve: {
extensions: ['.js'],
alias: {
'vs': path.resolve(outputPath, monacoEditorPath),
'dtrace-provider': emptyPath,
'safe-json-stringify': emptyPath,
'mv': emptyPath,
'source-map-support': emptyPath
'vs': path.resolve(outputPath, monacoEditorPath)
Copy link

Choose a reason for hiding this comment

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

Why is this not needed for bunyan anymore?

Copy link
Member Author

Choose a reason for hiding this comment

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

We don't use bunyan in the frontend and use the webpack to build only the frontend, so these modules are never bundled

Copy link

Choose a reason for hiding this comment

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

Right :) thx

}
},
devtool: 'source-map',
plugins: [
// @ts-ignore
new webpack.HotModuleReplacementPlugin(),
CopyWebpackPlugin([
new CopyWebpackPlugin([
{
from: requirePath,
to: '.'
Expand Down Expand Up @@ -124,8 +118,6 @@ module.exports = {
'*': 'http://' + host + ':' + port,
},
historyApiFallback: true,
hot: true,
inline: true,
stats: {
colors: true,
warnings: false
Expand Down
Empty file removed examples/browser/webpack_empty.js
Empty file.
4 changes: 1 addition & 3 deletions examples/electron/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ module.exports = {
filename: 'bundle.js',
path: outputPath,
libraryTarget: 'umd'

},
target: 'electron',
node: {
Expand Down Expand Up @@ -64,9 +63,8 @@ module.exports = {
},
devtool: 'source-map',
plugins: [
// @ts-ignore
new webpack.HotModuleReplacementPlugin(),
CopyWebpackPlugin([
new CopyWebpackPlugin([
{
from: monacoEditorPath,
to: 'vs'
Expand Down
11 changes: 8 additions & 3 deletions generator-theia/src/common/abstract-backend-generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,16 @@ function start() {
application.use(express.static(path.join(__dirname, '../../lib'), {
index: 'index.html'
}));
application.start();
return application.start(${this.ifWeb(`${this.model.config.port}, '${this.model.config.host}'`)})
}

Promise.resolve()${this.compileBackendModuleImports(backendModules)}
.then(start);`;
module.exports = Promise.resolve()${this.compileBackendModuleImports(backendModules)}
.then(start).catch(reason => {
console.error('Failed to start the backend application.');
if (reason) {
console.error(reason);
}
});`;
}

}
9 changes: 7 additions & 2 deletions generator-theia/src/common/abstract-frontend-generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,13 @@ function start() {
application.start();
}

Promise.resolve()${this.compileFrontendModuleImports(frontendModules)}
.then(start);`;
module.exports = Promise.resolve()${this.compileFrontendModuleImports(frontendModules)}
.then(start).catch(reason => {
console.error('Failed to start the frontend application.');
if (reason) {
console.error(reason);
}
});`;
}

}
18 changes: 17 additions & 1 deletion generator-theia/src/common/abstract-generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export abstract class AbstractGenerator {
return `Promise.resolve(${invocation})`;
}
return invocation;
}).map(statement => `.then(function () { return ${statement}.then(load) })`);
}).map(statement => ` .then(function () { return ${statement}.then(load) })`);
Copy link
Contributor

Choose a reason for hiding this comment

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

What is this change for?

Copy link
Member Author

Choose a reason for hiding this comment

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

to fix indentation in the generated source files: src-gen/frontend/main.js and src-gen/backend/main.js

Copy link

Choose a reason for hiding this comment

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

Yes thx for that! I had run into trouble with it

return os.EOL + lines.join(os.EOL);
}

Expand All @@ -60,4 +60,20 @@ export abstract class AbstractGenerator {
return copyright ? copyright + os.EOL : '';
}

protected isWeb(): boolean {
return this.model.target === 'web';
}

protected isElectron(): boolean {
return this.model.target === 'electron';
}

protected ifWeb(value: string, defaultValue: string = '') {
return this.isWeb() ? value : defaultValue;
}

protected ifElectron(value: string, defaultValue: string = '') {
return this.isElectron() ? value : defaultValue;
}

}
Loading