-
Notifications
You must be signed in to change notification settings - Fork 197
Conversation
354529d
to
2c0c9eb
Compare
c0189ba
to
bab1dac
Compare
@@ -2,6 +2,7 @@ const webpack = require("webpack"); | |||
const glob = require("glob"); | |||
const path = require("path"); | |||
const CopyWebpackPlugin = require('copy-webpack-plugin'); | |||
const HardSourceWebpackPlugin = require("hard-source-webpack-plugin"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we really need this? It added a lot of unwanted side effects
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now, when but in hard-source-webpack-plugin is fixed (now @0.13.1), everything seems to run well.
See,
@@ -10,7 +10,7 @@ import "../index"; | |||
|
|||
// tslint:disable:only-arrow-functions | |||
|
|||
const isNode = new Function("return typeof window === 'undefined' || this !== window")(); | |||
const isNode = typeof window === "undefined"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe the problem was that some packages tried to be clever and add a "window" global variable, breaking this check for node.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Define "some packages".
As far as i know, testing for process
is tricky as it's frequently used by e.q EnvironmentPlugin
and alike, but testing for window is good enough.
On the other hand, the old code didn't passed our yarn run tslint
check - and god knows how it succesfully passed CI.
zagorski@zagorski-dev-mee:~/here/harp.gl/mapsdk/coresdk$ yarn run tslint
yarn run v1.9.4
$ tslint --project tsconfig.json
ERROR: /home/zagorski/here/harp.gl/mapsdk/coresdk/@here/harp-fetch/test/FetchTest.ts:13:16 - Do not use the Function constructor to create functions.
ERROR: /home/zagorski/here/harp.gl/mapsdk/coresdk/@here/harp-mapview/lib/DebugContext.ts:39:16 - Do not use the Function constructor to create functions.
ERROR: /home/zagorski/here/harp.gl/mapsdk/coresdk/@here/harp-mapview/lib/DecodedTileHelpers.ts:148:5 - Interface has only a call signature — use `type ObjectConstructor = new (
geometry?: THREE.Geometry | THREE.BufferGeometry,
material?: THREE.Material
) => THREE.Object3D;` instead.
On the third hand, use of Function === eval, and we don't want it in code, even testing code. Unless really necessary (back to point 1).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sigh... Can't use process, because as soon as webpack sees it, it'll add some "convenience" code for you. As for "window" - make sure that there's no package in our stack that creates a global "window" variable (I forgot the details, but I believe there was at least one package that added "window" for node, that's why I added the additional this === window
check)
bab1dac
to
689e9d7
Compare
Common support for running tests (unit & integration) in browser using mocha-webdriver-runner. Initial support for firefox headless, with suspended support for headless Chrome due to bug [1]. [1]: puppeteer/puppeteer#3463
689e9d7
to
d561108
Compare
No description provided.