don't cross the browser and node streams

[othiym23]

My best case against conflating the two modes is this incredibly silly change I had to make to npm to get the affected files through standard. I know that window.opener is a thing in browsers, but it has not been and never will be part of node, and this error message (like the one you get for crypto in some contexts) makes no sense unless you know / care about the browser-side problem it's trying to address.

I tried very hard to figure out how to remove / override the relevant global check via code comments, but either I don't understand how eslint applies those overrides, or it's simply not supported. Either way, I get that a lot of code will be browserified at some point and thus should pass the browser checks, but there is no plausible or reasonable reason to run npm in the browser (phrased that way because I know somebody probably is going to try anyway at some point). It should be OK for server-side apps to say "I am a server-only thing".

[yoshuawuyts]

👍 io.js / node won't ever have a global window object (jsdom window is assigned explicitely) whereas the browser can have extra globals because of browserify. Being able to turn off browser to prevent issues like the one you just linked seems reasonable.

[othiym23]

Another good reason to split these modes apart: as I just discovered, because crypto is a global in the browser, using it without defining it first will cause standard to fail to notice that you haven't imported crypto in your Node module.

[feross]

Possible resolutions to this issue:

1. Leave things as they are. @othiym23 - you can tell eslint that opener is writable with a /*global opener:true */ comment at the top of the relevant file. Using /*global opener */ marks opener as read-only; use true to mark it writable. Not an ideal solution because it makes no sense to a node.js user why they should have to add this comment.
2. Keep the current default behavior, but add --node and --browser flags to restrict the assumed globals to only one environment's globals. Not ideal because we really, really want to avoid adding options. Like, that's sort of the point of standard.
3. Allow use of any browser global (like we already do), but mark them all as writable. This way, you can do var crypto = require('crypto') and var opener = require('opener') without errors in node. All the browser globals are defined here. This would eliminate all errors and annoyances, but not ideal because it's overly permissive: as @othiym23 mentioned, now standard won't error when any of the browser globals (like open, close, crypto, screen, scroll, stop, top) are accidentally used in node without being defined first.
4. Disallow use of browser globals except for window and document, as a compromise. These aren't really globals in node, so this is overly permissive, but at least it's just limited to TWO globals that we don't check. Then, force all browser code to reference browser globals as properties on window, e.g. window.XMLHttpRequest, window.open, window.crypto, etc. And document would just work: document.querySelector, etc. Not ideal because now browser code becomes wordier, and we're still not checking for improper use of these two variables in node (not a huge deal). Also, if standard is used with non-browserify browser code, it will still incorrectly assume that node globals like require and Buffer are defined. But I don't know how much we should care about non-browserify users.

Any other ideas?

[othiym23]

My preference is for 2, although I strongly agree with the goal of keeping configurability and modes to an absolute minimum. My argument in favor is that this is probably the biggest split going through the standard user community – many users are going to be writing code that is meant to work in both the browser and on the server, and smaller groups are going to be working on stuff that's meant for one or the other. The default behavior should stay the same as it is now, to support those users, but having a way to say, "yes, this is code that is only meant to run on the server" or "yes, this is code that is only meant to run in the browser" – that can be changed later – would be really helpful for those of us, like me, who are working with third-party libraries that make assumptions about the environment in which they're going to be used.

Just as a quick note, this would allow browserify users to get warnings for forgetting to shim their node globals in their browser code as well – no process or Buffer required? Whoops!

[mafintosh]

I think 4 is a nice middle ground. It is a bit annoying to not being able to use open as a variable name in node right now etc. Personally I prefix all global variables with window. in the browser anyways.

[jprichardson]

I'm vehemently against 2. User-friendly software should not have modes. 3 could cause problems. Although I already prefix all of my browser global variables with window, many don't - so I think 4 will discourage a lot of people from using standard. I realize that this is probably not an eslint option, but an ideal solution seems to be that if a variable is set with the result of a require statement, we could set it as writeable (eslint) as it would seem to me that the intent is pretty clear. This actually may not be that difficult, however it would add more processing time. The way that you'd do it is use something like recast (which in turn uses esprima), scan the source file for require statements that are set to browser globals, prepend the comment /*global opener:true */. If that's not viable, my vote would be to just stick with #1 (do nothing) for the time being.

The ultimate goal here is to try to encourage standard usage as much as possible, and 1 seems like a minor annoyance compared to the rest.

[Flet]

@jprichardson perhaps writing a custom eslint rule that replaces no-undef with this special treatment would not affect processing time as much?

[feross]

I thought of another option, 5:

• Whitelist window and document globals, as well as all browser globals that begin with an uppercase letter. Examples: Blob, DataView, and HTMLInputElement. Since they're upper case and usually pretty verbose, they're a lot less likely to be used as variable names in node than the lowercase browser globals. Lower case browser globals like length, open, top would need to be used as window.length, window.open, window.top.

Here's the full list of globals that we could keep whitelisted. Most don't seem like they would frequently cause problems in node.

[ 'Audio',
  'AudioProcessingEvent',
  'BeforeUnloadEvent',
  'Blob',
  'CanvasGradient',
  'CanvasPattern',
  'CanvasRenderingContext2D',
  'CloseEvent',
  'Comment',
  'CompositionEvent',
  'CSS',
  'CustomEvent',
  'DataView',
  'Debug',
  'Document',
  'DocumentFragment',
  'DOMParser',
  'DragEvent',
  'Element',
  'ElementTimeControl',
  'ErrorEvent',
  'Event',
  'FileReader',
  'FocusEvent',
  'FormData',
  'GamepadEvent',
  'HashChangeEvent',
  'HTMLAnchorElement',
  'HTMLBaseElement',
  'HTMLBlockquoteElement',
  'HTMLBodyElement',
  'HTMLBRElement',
  'HTMLButtonElement',
  'HTMLCanvasElement',
  'HTMLDirectoryElement',
  'HTMLDivElement',
  'HTMLDListElement',
  'HTMLElement',
  'HTMLFieldSetElement',
  'HTMLFontElement',
  'HTMLFormElement',
  'HTMLFrameElement',
  'HTMLFrameSetElement',
  'HTMLHeadElement',
  'HTMLHeadingElement',
  'HTMLHRElement',
  'HTMLHtmlElement',
  'HTMLIFrameElement',
  'HTMLImageElement',
  'HTMLInputElement',
  'HTMLIsIndexElement',
  'HTMLLabelElement',
  'HTMLLayerElement',
  'HTMLLegendElement',
  'HTMLLIElement',
  'HTMLLinkElement',
  'HTMLMapElement',
  'HTMLMenuElement',
  'HTMLMetaElement',
  'HTMLModElement',
  'HTMLObjectElement',
  'HTMLOListElement',
  'HTMLOptGroupElement',
  'HTMLOptionElement',
  'HTMLParagraphElement',
  'HTMLParamElement',
  'HTMLPreElement',
  'HTMLQuoteElement',
  'HTMLScriptElement',
  'HTMLSelectElement',
  'HTMLStyleElement',
  'HTMLTableCaptionElement',
  'HTMLTableCellElement',
  'HTMLTableColElement',
  'HTMLTableElement',
  'HTMLTableRowElement',
  'HTMLTableSectionElement',
  'HTMLTextAreaElement',
  'HTMLTitleElement',
  'HTMLUListElement',
  'HTMLVideoElement',
  'IDBCursor',
  'IDBCursorWithValue',
  'IDBDatabase',
  'IDBEnvironment',
  'IDBFactory',
  'IDBIndex',
  'IDBKeyRange',
  'IDBObjectStore',
  'IDBOpenDBRequest',
  'IDBRequest',
  'IDBTransaction',
  'IDBVersionChangeEvent',
  'Image',
  'InputEvent',
  'Intl',
  'KeyboardEvent',
  'MessageChannel',
  'MessageEvent',
  'MessagePort',
  'MouseEvent',
  'MutationObserver',
  'Node',
  'NodeFilter',
  'NodeList',
  'Notification',
  'OfflineAudioCompletionEvent',
  'Option',
  'PageTransitionEvent',
  'PopStateEvent',
  'ProgressEvent',
  'Range',
  'SharedWorker',
  'StorageEvent',
  'SVGAElement',
  'SVGAltGlyphDefElement',
  'SVGAltGlyphElement',
  'SVGAltGlyphItemElement',
  'SVGAngle',
  'SVGAnimateColorElement',
  'SVGAnimatedAngle',
  'SVGAnimatedBoolean',
  'SVGAnimatedEnumeration',
  'SVGAnimatedInteger',
  'SVGAnimatedLength',
  'SVGAnimatedLengthList',
  'SVGAnimatedNumber',
  'SVGAnimatedNumberList',
  'SVGAnimatedPathData',
  'SVGAnimatedPoints',
  'SVGAnimatedPreserveAspectRatio',
  'SVGAnimatedRect',
  'SVGAnimatedString',
  'SVGAnimatedTransformList',
  'SVGAnimateElement',
  'SVGAnimateMotionElement',
  'SVGAnimateTransformElement',
  'SVGAnimationElement',
  'SVGCircleElement',
  'SVGClipPathElement',
  'SVGColor',
  'SVGColorProfileElement',
  'SVGColorProfileRule',
  'SVGComponentTransferFunctionElement',
  'SVGCSSRule',
  'SVGCursorElement',
  'SVGDefsElement',
  'SVGDescElement',
  'SVGDocument',
  'SVGElement',
  'SVGElementInstance',
  'SVGElementInstanceList',
  'SVGEllipseElement',
  'SVGEvent',
  'SVGExternalResourcesRequired',
  'SVGFEBlendElement',
  'SVGFEColorMatrixElement',
  'SVGFEComponentTransferElement',
  'SVGFECompositeElement',
  'SVGFEConvolveMatrixElement',
  'SVGFEDiffuseLightingElement',
  'SVGFEDisplacementMapElement',
  'SVGFEDistantLightElement',
  'SVGFEFloodElement',
  'SVGFEFuncAElement',
  'SVGFEFuncBElement',
  'SVGFEFuncGElement',
  'SVGFEFuncRElement',
  'SVGFEGaussianBlurElement',
  'SVGFEImageElement',
  'SVGFEMergeElement',
  'SVGFEMergeNodeElement',
  'SVGFEMorphologyElement',
  'SVGFEOffsetElement',
  'SVGFEPointLightElement',
  'SVGFESpecularLightingElement',
  'SVGFESpotLightElement',
  'SVGFETileElement',
  'SVGFETurbulenceElement',
  'SVGFilterElement',
  'SVGFilterPrimitiveStandardAttributes',
  'SVGFitToViewBox',
  'SVGFontElement',
  'SVGFontFaceElement',
  'SVGFontFaceFormatElement',
  'SVGFontFaceNameElement',
  'SVGFontFaceSrcElement',
  'SVGFontFaceUriElement',
  'SVGForeignObjectElement',
  'SVGGElement',
  'SVGGlyphElement',
  'SVGGlyphRefElement',
  'SVGGradientElement',
  'SVGHKernElement',
  'SVGICCColor',
  'SVGImageElement',
  'SVGLangSpace',
  'SVGLength',
  'SVGLengthList',
  'SVGLinearGradientElement',
  'SVGLineElement',
  'SVGLocatable',
  'SVGMarkerElement',
  'SVGMaskElement',
  'SVGMatrix',
  'SVGMetadataElement',
  'SVGMissingGlyphElement',
  'SVGMPathElement',
  'SVGNumber',
  'SVGNumberList',
  'SVGPaint',
  'SVGPathElement',
  'SVGPathSeg',
  'SVGPathSegArcAbs',
  'SVGPathSegArcRel',
  'SVGPathSegClosePath',
  'SVGPathSegCurvetoCubicAbs',
  'SVGPathSegCurvetoCubicRel',
  'SVGPathSegCurvetoCubicSmoothAbs',
  'SVGPathSegCurvetoCubicSmoothRel',
  'SVGPathSegCurvetoQuadraticAbs',
  'SVGPathSegCurvetoQuadraticRel',
  'SVGPathSegCurvetoQuadraticSmoothAbs',
  'SVGPathSegCurvetoQuadraticSmoothRel',
  'SVGPathSegLinetoAbs',
  'SVGPathSegLinetoHorizontalAbs',
  'SVGPathSegLinetoHorizontalRel',
  'SVGPathSegLinetoRel',
  'SVGPathSegLinetoVerticalAbs',
  'SVGPathSegLinetoVerticalRel',
  'SVGPathSegList',
  'SVGPathSegMovetoAbs',
  'SVGPathSegMovetoRel',
  'SVGPatternElement',
  'SVGPoint',
  'SVGPointList',
  'SVGPolygonElement',
  'SVGPolylineElement',
  'SVGPreserveAspectRatio',
  'SVGRadialGradientElement',
  'SVGRect',
  'SVGRectElement',
  'SVGRenderingIntent',
  'SVGScriptElement',
  'SVGSetElement',
  'SVGStopElement',
  'SVGStringList',
  'SVGStylable',
  'SVGStyleElement',
  'SVGSVGElement',
  'SVGSwitchElement',
  'SVGSymbolElement',
  'SVGTests',
  'SVGTextContentElement',
  'SVGTextElement',
  'SVGTextPathElement',
  'SVGTextPositioningElement',
  'SVGTitleElement',
  'SVGTransform',
  'SVGTransformable',
  'SVGTransformList',
  'SVGTRefElement',
  'SVGTSpanElement',
  'SVGUnitTypes',
  'SVGURIReference',
  'SVGUseElement',
  'SVGViewElement',
  'SVGViewSpec',
  'SVGVKernElement',
  'SVGZoomAndPan',
  'Text',
  'TextDecoder',
  'TextEncoder',
  'TimeEvent',
  'TouchEvent',
  'UIEvent',
  'URL',
  'WebGLActiveInfo',
  'WebGLBuffer',
  'WebGLContextEvent',
  'WebGLFramebuffer',
  'WebGLProgram',
  'WebGLRenderbuffer',
  'WebGLRenderingContext',
  'WebGLShader',
  'WebGLShaderPrecisionFormat',
  'WebGLTexture',
  'WebGLUniformLocation',
  'WebSocket',
  'WheelEvent',
  'Window',
  'Worker',
  'XDomainRequest',
  'XMLHttpRequest',
  'XMLSerializer',
  'XPathEvaluator',
  'XPathException',
  'XPathExpression',
  'XPathNamespace',
  'XPathNSResolver',
  'XPathResult' ]

These variables, on the other hand, would need to be accessed on window, like window.btoa:

[ 'addEventListener',
  'alert',
  'applicationCache',
  'atob',
  'blur',
  'btoa',
  'cancelAnimationFrame',
  'clearInterval',
  'clearTimeout',
  'close',
  'closed',
  'confirm',
  'console',
  'crypto',
  'defaultStatus',
  'devicePixelRatio',
  'dispatchEvent',
  'document',
  'event',
  'find',
  'focus',
  'frameElement',
  'frames',
  'getComputedStyle',
  'getSelection',
  'history',
  'indexedDB',
  'innerHeight',
  'innerWidth',
  'length',
  'localStorage',
  'location',
  'matchMedia',
  'moveBy',
  'moveTo',
  'name',
  'navigator',
  'onbeforeunload',
  'onblur',
  'onerror',
  'onfocus',
  'onload',
  'onresize',
  'onunload',
  'open',
  'openDatabase',
  'opener',
  'opera',
  'outerHeight',
  'outerWidth',
  'pageXOffset',
  'pageYOffset',
  'parent',
  'postMessage',
  'print',
  'prompt',
  'removeEventListener',
  'requestAnimationFrame',
  'resizeBy',
  'resizeTo',
  'screen',
  'screenX',
  'screenY',
  'scroll',
  'scrollbars',
  'scrollBy',
  'scrollTo',
  'scrollX',
  'scrollY',
  'self',
  'sessionStorage',
  'setInterval',
  'setTimeout',
  'showModalDialog',
  'status',
  'stop',
  'top',
  'window' ]

[feross]

Btw, I'm running option 4 on a local branch – and I really like it so far – the tests all pass with just a few additions of window. or global. in browser code and we don't need to add modes.

[feross]

I just published 3.0.0-alpha with option 4. Please give it a try and share your feedback! It resolves @othiym23's original issue without introducing new options. The only exception is that window and document are always assumed to exist, even in node. Small issue, IMO.

npm install standard@3.0.0-alpha -g

[feross]

Wow, eslint just made a change to properly support CommonJS module scope, i.e. using a variable name that is the same as a global is now allowed! eslint now understands that the local variable shadows the global in node (which we allow), and the "redefining a read-only variable" error goes away!

eslint/eslint#892

This is basically approach 3.

[QuantumInformation]

I'm getting 'CustomEvent' is not defined. Will this be added to the whitelist?

Using "standard": "^9.0.2",

[yoshuawuyts]

Try `window.CustomEvent` instead

…

On Fri, Mar 31, 2017 at 1:00 PM Nikos ***@***.***> wrote: I'm getting 'CustomEvent' is not defined. Will this be added to the whitelist? — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#46 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACWlep2kj_YSZArnP-kFQimdKd1_x7FLks5rrNzDgaJpZM4Dmlfc> .

[QuantumInformation]

I just done this:

  "standard": {
    "globals": [
      "CustomEvent",
      "Event"
    ]
  }