Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow nullish coalescing operator (??) in Javascript #3682

Open
wants to merge 2 commits into
base: dev-11.0
Choose a base branch
from

Conversation

ThoWagen
Copy link
Contributor

This PR changes the eslint and jshint configuration so that the nullish coalescing operator (??) is allowed in Javascript.

Copy link
Contributor

@EreMaijala EreMaijala left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Worth noting that this change will allow a lot more than just the nullish coalescing operator.

I'm slightly worried that this will exclude a significant number of older devices that cannot be updated (e.g. iPads) that, while obviously outdated and probably not very safe, are otherwise still usable. I'm not saying this change shouldn't be made, but dropping the support for pre-2020 devices should be a conscious decision.

I'm saying this because we had a significant number of problem reports when we enabled cookie consent with a library version that required support for the optional chaining operator. It's been a while since, so the situation might be different.

.eslintrc.js Outdated
@@ -10,6 +10,9 @@ module.exports = {
"es6": true,
"jquery": true
},
parserOptions: {
"ecmaVersion": "latest"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this doesn't actually change anything since latest has been the default, but should we actually use something like 2020 to align with esversion with jshint?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think that makes sense. Changed it.

@ThoWagen
Copy link
Contributor Author

ThoWagen commented May 22, 2024

I understand your concern. So, I would not mind waiting with this change a bit longer.

The nullish coalescing operator would work really well for adding default values to the new js configs. But we also could add default values with an additional parameter for the get method.

You and @demiankatz have more experience and know the user base better than me so I trust your judgement.

@crhallberg
Copy link
Contributor

crhallberg commented May 22, 2024

I wouldn't want to change device support without a bit of a heads up. So we could sync this with a future release like 10.1 or 11.

@demiankatz demiankatz added this to the 11.0 milestone May 22, 2024
@demiankatz
Copy link
Member

I'm putting the milestone on this at 11.0, since this feels like a "major version" change, and I don't think we have enough time to thoroughly finish this conversation in time for release 10.0.

We do have a browser support statement, so I think we'd have to update that if we merged this. The question is whether we want to have a more specific policy going forward (e.g. "last five years of hardware") to help make these kinds of decisions easier. Probably worth a future Community Call discussion, which we should have time for over the summer, after the push for release 10.0 is over.

@demiankatz demiankatz changed the base branch from dev to dev-11.0 August 27, 2024 10:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants