BREAKING CHANGE: Lerna/monorepo refactor package split #199
Conversation
Split into 3 packages: - @morgan-stanley/desktopjs - @morgan-stanley/desktopjs-electron - @morgan-stanley/desktpojs-openfin
| @@ -3,7 +3,10 @@ | |||
| "version": "0.0.0", | |||
| "description": "desktopJS Demo", | |||
| "main": "electron.js", | |||
| "private": true, | |||
There was a problem hiding this comment.
This is one of the lerna packages so the desktopjs symlink works but we don't want it published.
package.json
Outdated
| "gulp-typedoc": "^2.0.2", | ||
| "gulp-typescript": "^4.0.1", | ||
| "gulp-uglify": "^3.0.0", | ||
| "gulp-typescript": "^5.0.0-alpha.3", |
There was a problem hiding this comment.
Just noticing this. It must have been tagged as latest. If there are any objects I'll manually pin but greenkeeper will probably try and upgrade.
There was a problem hiding this comment.
yeah, it should be a non alpha at least
| @@ -0,0 +1,3 @@ | |||
| # @morgan-stanley/desktopjs-electron | |||
There was a problem hiding this comment.
This is mostly a placeholder readme. I'm debating whether I'll have info here or just link to the main project as-is.
| @@ -231,10 +230,10 @@ export class ElectronContainerWindow extends ContainerWindow { | |||
| * @augments MessageBus | |||
| */ | |||
| export class ElectronMessageBus implements MessageBus { | |||
| public ipc: NodeJS.EventEmitter; | |||
| public ipc: any; | |||
There was a problem hiding this comment.
Getting rid of exposed node types to avoid upstream pain when devs attempt to build.
There was a problem hiding this comment.
we should mention it in release notes as a breaking change
There was a problem hiding this comment.
Will be in release notes
| @@ -1,20 +1,24 @@ | |||
| { | |||
| "compilerOptions": { | |||
| "target": "es5", | |||
| "target": "es6", | |||
There was a problem hiding this comment.
This is staging project used for tests and dts generation, not the actual build which is still es5/rollup -> umd.
| @@ -0,0 +1,3 @@ | |||
| # @morgan-stanley/desktopjs-openfin | |||
There was a problem hiding this comment.
This is mostly a placeholder readme. I'm debating whether I'll have info here or just link to the main project as-is.
| private uuid: string; | ||
|
|
||
| public constructor(bus: fin.OpenFinInterApplicationBus, uuid: string) { | ||
| public constructor(bus: any, uuid: string) { |
There was a problem hiding this comment.
Avoid exposing OpenFin types to alleviate pain of upstream consumer.
There was a problem hiding this comment.
same as above, we should add this to release notes
There was a problem hiding this comment.
Will be in release notes
| describe("createWindow", () => { | ||
| beforeEach(() => { | ||
| spyOn(desktop, "Window").and.callFake((options?: any, callback?: Function) => { if (callback) { callback(); } }); | ||
| }); | ||
|
|
||
| it("defaults", (done) => { | ||
| xit("defaults", (done) => { |
There was a problem hiding this comment.
This is part of the WIP.
| container.createWindow("url").then(win => { | ||
| expect(win).toBeDefined(); | ||
| expect(desktop.Window).toHaveBeenCalledWith({ autoShow: true, url: "url", name: jasmine.stringMatching(/\w{8}-\w{4}-\w{4}-\w{4}-\w{12}/), customData: undefined }, jasmine.any(Function), jasmine.any(Function)); | ||
| done(); | ||
| }); | ||
| }); | ||
|
|
||
| it("createWindow defaults", (done) => { | ||
| xit("createWindow defaults", (done) => { |
There was a problem hiding this comment.
This is part of the WIP.
|
|
||
| // @ts-ignore |
There was a problem hiding this comment.
ts-ignore in tests are part of WIP
There was a problem hiding this comment.
Fixed (except for mock classes not implementing all members)
| @@ -8,7 +8,7 @@ | |||
| */ | |||
| "insecure-random": true, | |||
| "no-banned-terms": true, | |||
| "no-cookies": true, | |||
| "no-cookies": false, | |||
There was a problem hiding this comment.
These have always been displayed as warning as not being supported with current typescript flags. Turning off enforcement to remove warnings.
Codecov Report
@@ Coverage Diff @@
## master #199 +/- ##
==========================================
- Coverage 93.49% 93.15% -0.34%
==========================================
Files 15 15
Lines 1444 1418 -26
Branches 244 246 +2
==========================================
- Hits 1350 1321 -29
- Misses 94 97 +3
|
|
Loss in coverage is from WIP xit tests for create window in electron/openfin packages. |
|
I'm OK to approve when the WIPs and the other few comments are handled 👍 |
Split into 3 packages: - @morgan-stanley/desktopjs - @morgan-stanley/desktopjs-electron - @morgan-stanley/desktpojs-openfin
bingenito
changed the title
Split into 3 packages:
Fixes #196