AMD/CommonJS/Globals #12909

[fat]

the following jshint tests are failing:

• Missing "use strict" statement.
• 'define' is not defined.
• 'require' is not defined.

I dont want to add "use strict" to this factory method. If that means removing the js hint rule for that, im fine with that.

Not sure where to add our define/require exports… @cvrebert any idea?

[XhmikosR]

You can just disable the warning for that code block...

[fat]

i dont think we need the strict rule because we follow a template with all our plugins. the top of them is copy pasted and completely consistent across the board.

i also had to disable requireCamelCaseOrUpperCaseIdentifiers because o_o is an adorable function name and much better than "factory".

i went back and forth with disallowSpacesInsideParentheses… because ( o_o) looks better to me than (o_o) … but i do think that rule is kinda nice to catch accidental spaces in other parentheses

[fat]

could be convinced of the validity of disallowSpacesInsideParentheses

[fat]

but also kinda meh about it

[XhmikosR]

So you basically want to revert every change everyone else made... Oh well, I guess I'll stop bothering with the JS part anymore. No worries.

[mdo]

@XhmikosR I don't think @fat's changes are that far removed from what we have today. They certainly aren't reverting everyone else's changes, are they?

[hnrch02]

I don't think we should remove any rules that ensure consistency across the JavaScript because of an adorable function name. I mean it is cute, but IMHO not worth it.
But 👍 on the actual UMD stuff.

[cvrebert]

Yeah, let's avoid the gratuitously opaque/weird function name.

[cvrebert]

Not sure where to add our define/require exports… @cvrebert any idea?

I haven't used AMD/CommonJS before, so I have no idea.

[XhmikosR]

@mdo: it is reverting the changes I made in the other PR, which had like 3/4 reverted already. No worries, I don't mind, I'll leave others bother with JS.

[thebyrd]

:)

[ox]

:)

[cvrebert]

ಠ_ಠ

[fat]

just turning off strict mode and requireCamelCaseOrUpperCaseIdentifiers which aren't hard to miss things. We obviously always camel case our identifiers (except in this case, but it's very intentional) and strict mode isn't something i'm worried about us missing in a plugin.

[hnrch02]

You could have just added

/* jshint strict: false */

to the body of the factory function.

[hnrch02]

[...] which aren't hard to miss things.

That's true for us, but having undocumented/unenforced rules about our coding style makes contributing harder. If somebody were to submit a PR and use an identifier that's not in camelCase, the style check that should report such problems passes and the error can't be resolved until somebody from the team points this out to the contributor. Not that fatal, as PRs are properly reviewed by us before they're merged, but given that the tool we use for code style checking has this feature, I think it's counter-intuitive not to use it. Even more so, as we're removing this because of an "gratuitously opaque/weird" function name.

Anyways, just my 2¢.

[hnrch02]

This also introduced regression of #10038.

[cvrebert]

@hnrch02 File new issue please.

[hnrch02]

@cvrebert I'm preparing a PR right now.