Remove HTMLHint from being require()'d and instead use window global like other linters#4962
Remove HTMLHint from being require()'d and instead use window global like other linters#4962westonruter wants to merge 1 commit intocodemirror:masterfrom
Conversation
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. |
|
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:
In the second case, it could look like doing the following after calling CodeMirror.lint.html.HTMLHint = HTMLHint;Then in WordPress, for example, a plugin could do: CodeMirror.lint.html.HTMLHint.addRule( /* … */ );Thoughts? |
|
I just found something that may add weight for opting for a global Nevertheless, I've found a solution to continue opting for
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. |
Couldn't those plugins simply also require it?
I agree. Want to submit a pull request? |
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 would, but I won't be able to for some time. If someone else wants to contribute this I'm happy if they do. |
Add a statement like Closing this, looking forward to further pull requests. |
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 nowindow.HTMLHintfor 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.HTMLHintas well.