Add babel-plugin-probe-gl module #111
Conversation
|
|
||
| Remove calls to specified logging methods. This is useful to reduce the logging system's footprint and performance impact in production environment. | ||
|
|
||
| The `Log` instance must be called `log` for this feature to work. |
There was a problem hiding this comment.
Can this be an option that's passed in.
| ```json | ||
| { | ||
| "plugins": [["probe-gl", { | ||
| "removeLogs": ["log", "warn"], |
There was a problem hiding this comment.
Would it be hard to make this removeMethods so you can remove whatever code you want?
{
"plugins": [["probe-gl", {
"removeMethods": {
"object": "myLogger",
"methods": ["log", "warn", "myLogging"],
},
"patterns": ["*.glsl.js"]
}]]
}
There was a problem hiding this comment.
This looks very good, added some thoughts on naming and generalization.
PS - Another dev-module that should move to probe.gl is the eslint checker for "double parenthesis on probe.gl log calls" that is currently in luma.gl.
https://github.com/uber/luma.gl/tree/master/dev-modules/eslint-plugin-luma-gl-custom-rules.
I think that could be called eslint-plugin-probe-gl
| "extends": "node_modules/ocular-dev-tools/templates/.nycrc" | ||
| "extends": "node_modules/ocular-dev-tools/templates/.nycrc", | ||
| "include": [ | ||
| "dev-modules/**/*.js", |
There was a problem hiding this comment.
Nit: Worth avoiding different directory structures between modules and dev-modules?
Even if dev-modules often just have one source file, just keep the same structure to follow the principle of least surprises, and so we can move them between different modules and dev-modules directories in the future?
|
|
||
| ## Usage | ||
|
|
||
| ### Via `.babelrc` (Recommended) |
There was a problem hiding this comment.
Total nit: maybe babel.config.js is more common these days...
| @@ -0,0 +1,22 @@ | |||
| { | |||
| "name": "babel-plugin-probe-gl", | |||
There was a problem hiding this comment.
On the topic of naming,
- Do we expect to have only one plugin for probe, possibly doing multiple things in the future, or do we want to qualify the name with what the plugin does, e.g.
babel-plugin-probe.gl-strip-logs? - I expect it is not worth the hassle to use the probe.gl namespace on a babel plugin
@probe.gl/babel-plugin-strip-logs)
There was a problem hiding this comment.
As for extensions to this, I am interested in having a babel-plugin-strip-assert:
- The existing npm module for this does not seem to work or be maintained. I expect a small variant of this plugin could work for that case.
- We could just make one plugin
babel-plugin-strip-functionsthat handles both functions and methods. - The main weirdness is that generic plugin would come with probe.gl specific defaults, but we can always call it
babel-plugin-probe.gl-strip-functionsto get around that.
| // Remove whitespace before comparing | ||
| function clean(code) { | ||
| return code | ||
| .replace('"use strict";', '') |
There was a problem hiding this comment.
Assume babel injects this in the transpiled code?
| const config = { | ||
| lint: { | ||
| paths: ['modules', 'examples', 'test'], | ||
| extensions: ['js'] | ||
| }, | ||
|
|
||
| aliases: { | ||
| // DEV MODULES | ||
| 'dev-modules': resolve(__dirname, './dev-modules') |
There was a problem hiding this comment.
Nit: Maybe alias the actual plugin module instead?
|
Xiaoji Chen seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
See README.md