Remove HTMLHint from being require()'d and instead use window global like other linters

[westonruter]

I'm working on integrating CodeMirror into WordPress core to be part of the November release of 4.9. The work is being done in the Better Code Editing feature plugin. In order to improve performance of loading CodeMirror, we needed to create a bundle of the assets that we're going to be using. See WordPress/better-code-editing#92 if helpful.

When using Browserify, however, I came across a problem where HTMLHint was being imported into the bundle unlike the other linting libraries which all look at window. When HTMLHint is imported into the bundle, then there is no window.HTMLHint for other scripts to interface with. This is a particular problem in this case because the plugin adds a new rule to HTMLHint to flag HTML as invalid if the user does not have permission (e.g. to add scripts).

So I suggest, to better align with all of the other linters, that CodeMirror defer to window.HTMLHint as well.

[marijnh]

then there is no window.HTMLHint for other scripts to interface with.

Isn't that a good thing? Many of the linters aren't distributed as CommonJS, so they use globals because they have no choice. But integrating with module loaders is definitely preferred.

[westonruter]

Then in the case of an extension wanting to add a custom rule to HTMLHint or other linters that get imported, the two other options I see would be:

1. Include extensions in the build, keeping everything private. This won't be feasible for 3rd-party plugins.
2. Expose the HTMLHint the object on CodeMirror for plugins to extend.

In the second case, it could look like doing the following after calling CodeMirror.registerHelper("lint", "html", … ):

CodeMirror.lint.html.HTMLHint = HTMLHint;

Then in WordPress, for example, a plugin could do:

CodeMirror.lint.html.HTMLHint.addRule( /* … */ );

Thoughts?

[westonruter]

I just found something that may add weight for opting for a global window.HTMLHint: the HTMLHint library itself will require('jshint') and require('csslint'). Because of this, private copies of both JSHint and CSSLint will be present in the browserified CodeMirror bundle and will be used by the HTMLHint rules for jshint and csslint, whereas CodeMirror's lint addons for JS and CSS will use the separate copies on the window object.

Nevertheless, I've found a solution to continue opting for window.HTMLHint even when bundling via the browserify-shim module.

Many of the linters aren't distributed as CommonJS, so they use globals because they have no choice.

It seems that for JSHint, CSSLint, and JSONLint that these available on NPM as CommonJS modules and could be updated to use what CodeMirror is doing for HTMLHint if that is the desired path instead of relying on globals.

[marijnh]

Expose the HTMLHint the object on CodeMirror for plugins to extend.

Couldn't those plugins simply also require it?

It seems that for JSHint, CSSLint, and JSONLint that these available on NPM as CommonJS modules and could be updated to use what CodeMirror is doing for HTMLHint if that is the desired path instead of relying on globals.

I agree. Want to submit a pull request?

[westonruter]

Couldn't those plugins simply also require it?

Not really, because the CodeMirror bundle would be shipping with core. A plugin would ship with its own assets and it would not be able to rebuild the bundle that is in core. If a plugin were to require the codemirror assets as part of its own build process and then replace the bundle that is enqueued in core, this is a possibility. However, it would mean only one plugin at a time would be able to extend CodeMirror in this way, since each plugin would be competing to override the core bundle of CodeMirror with their own extended bundles.

I agree. Want to submit a pull request?

I would, but I won't be able to for some time. If someone else wants to contribute this I'm happy if they do.

[marijnh]

Not really, because the CodeMirror bundle would be shipping with core.

Add a statement like window.HTMLHint = require("htmlhint") to your bundle.

Closing this, looking forward to further pull requests.