Skip to content

ref(tracing): Remove nullish coalescing operator and add eslint rule #6783

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

Merged
merged 3 commits into from
Jan 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/eslint-config-sdk/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,10 @@ module.exports = {
},
],

// We want to prevent optional chaining & nullish coalescing usage in our files
// to prevent uncessary bundle size. Turned off in tests.
'@sentry-internal/sdk/no-optional-chaining': 'error',
'@sentry-internal/sdk/no-nullish-coalescing': 'error',

// JSDOC comments are required for classes and methods. As we have a public facing codebase, documentation,
// even if it may seems excessive at times, is important to emphasize. Turned off in tests.
Expand Down Expand Up @@ -177,6 +180,7 @@ module.exports = {
'@typescript-eslint/no-non-null-assertion': 'off',
'@typescript-eslint/no-empty-function': 'off',
'@sentry-internal/sdk/no-optional-chaining': 'off',
'@sentry-internal/sdk/no-nullish-coalescing': 'off',
},
},
{
Expand Down
1 change: 1 addition & 0 deletions packages/eslint-plugin-sdk/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
module.exports = {
rules: {
'no-optional-chaining': require('./rules/no-optional-chaining'),
'no-nullish-coalescing': require('./rules/no-nullish-coalescing'),
'no-eq-empty': require('./rules/no-eq-empty'),
},
};
48 changes: 48 additions & 0 deletions packages/eslint-plugin-sdk/src/rules/no-nullish-coalescing.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
/**
* @fileoverview disallow nullish coalescing operators as they were introduced only in ES2020 and hence require
* us to add a polyfill. This increases bundle size more than avoiding nullish coalescing operators all together.
*
* @author Lukas Stracke
*
* Based on: https://github.com/mysticatea/eslint-plugin-es/blob/v4.1.0/lib/rules/no-nullish-coalescing-operators.js
*/
'use strict';

// ------------------------------------------------------------------------------
// Rule Definition
// ------------------------------------------------------------------------------

module.exports = {
meta: {
type: 'problem',
docs: {
description: 'disallow nullish coalescing operators.',
category: 'Best Practices',
recommended: true,
},
messages: {
forbidden: 'Avoid using nullish coalescing operators.',
},
fixable: null,
schema: [],
},
create(context) {
return {
"LogicalExpression[operator='??']"(node) {
context.report({
node: context.getSourceCode().getTokenAfter(node.left, isNullishCoalescingOperator),
messageId: 'forbidden',
});
},
};
},
};

/**
* Checks if the given token is a nullish coalescing operator or not.
* @param {Token} token - The token to check.
* @returns {boolean} `true` if the token is a nullish coalescing operator.
*/
function isNullishCoalescingOperator(token) {
return token.value === '??' && token.type === 'Punctuator';
}
1 change: 1 addition & 0 deletions packages/nextjs/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ module.exports = {
extends: ['../../.eslintrc.js'],
rules: {
'@sentry-internal/sdk/no-optional-chaining': 'off',
'@sentry-internal/sdk/no-nullish-coalescing': 'off',
},
overrides: [
{
Expand Down
1 change: 1 addition & 0 deletions packages/node/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,6 @@ module.exports = {
extends: ['../../.eslintrc.js'],
rules: {
'@sentry-internal/sdk/no-optional-chaining': 'off',
'@sentry-internal/sdk/no-nullish-coalescing': 'off',
},
};
2 changes: 1 addition & 1 deletion packages/tracing/src/browser/browsertracing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ export class BrowserTracing implements Integration {
op,
trimEnd: true,
metadata: {
source: this._latestRouteSource ?? 'url',
source: this._latestRouteSource || 'url',
},
};

Expand Down
2 changes: 1 addition & 1 deletion packages/tracing/src/browser/metrics/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ function _addPerformanceNavigationTiming(
}
_startChild(transaction, {
op: 'browser',
description: description ?? event,
description: description || event,
startTimestamp: timeOrigin + msToSec(start),
endTimestamp: timeOrigin + msToSec(end),
});
Expand Down
10 changes: 5 additions & 5 deletions packages/tracing/src/span.ts
Original file line number Diff line number Diff line change
Expand Up @@ -294,17 +294,17 @@ export class Span implements SpanInterface {
* @inheritDoc
*/
public updateWithContext(spanContext: SpanContext): this {
this.data = spanContext.data ?? {};
this.data = spanContext.data || {};
this.description = spanContext.description;
this.endTimestamp = spanContext.endTimestamp;
this.op = spanContext.op;
this.parentSpanId = spanContext.parentSpanId;
this.sampled = spanContext.sampled;
this.spanId = spanContext.spanId ?? this.spanId;
this.startTimestamp = spanContext.startTimestamp ?? this.startTimestamp;
this.spanId = spanContext.spanId || this.spanId;
this.startTimestamp = spanContext.startTimestamp || this.startTimestamp;
this.status = spanContext.status;
this.tags = spanContext.tags ?? {};
this.traceId = spanContext.traceId ?? this.traceId;
this.tags = spanContext.tags || {};
this.traceId = spanContext.traceId || this.traceId;

return this;
}
Expand Down
2 changes: 1 addition & 1 deletion packages/tracing/src/transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ export class Transaction extends SpanClass implements TransactionInterface {
public updateWithContext(transactionContext: TransactionContext): this {
super.updateWithContext(transactionContext);

this.name = transactionContext.name ?? '';
this.name = transactionContext.name || '';

this._trimEnd = transactionContext.trimEnd;

Expand Down