Skip to content

Commit

Permalink
Do not allow arrow functions as the "factory" function for registerSe…
Browse files Browse the repository at this point in the history
…rviceDorDoc (#26776)

* don't allow registration of arrow function as factory

* apply lint fixes

* Do not allow arrow functions as the "factory" function
  • Loading branch information
erwinmombay committed Feb 13, 2020
1 parent 75b44bb commit 09d4985
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 3 deletions.
1 change: 1 addition & 0 deletions .eslintrc
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,7 @@
"local/is-experiment-on": 2,
"local/json-configuration": 2,
"local/no-array-destructuring": 2,
"local/no-arrow-on-register-functions": 2,
"local/no-deep-destructuring": 2,
"local/no-destructure-assignment-params": 2,
"local/no-duplicate-import": 2,
Expand Down
32 changes: 32 additions & 0 deletions build-system/eslint-rules/no-arrow-on-register-functions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/**
* Copyright 2020 The AMP HTML Authors. All Rights Reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS-IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
'use strict';

const expression = 'CallExpression[callee.property.name=registerServiceForDoc]';
module.exports = function(context) {
return {
[expression]: function(node) {
if (node.arguments[1].type === 'ArrowFunctionExpression') {
// TODO(erwinm): add fixer method.
context.report({
node,
message:
'The 2nd argument of registerServiceForDoc should not be an arrow function.',
});
}
},
};
};
4 changes: 3 additions & 1 deletion extensions/amp-geo/0.1/amp-geo.js
Original file line number Diff line number Diff line change
Expand Up @@ -425,5 +425,7 @@ let geoDeferred = null;
AMP.extension('amp-geo', '0.1', AMP => {
geoDeferred = new Deferred();
AMP.registerElement(TAG, AmpGeo);
AMP.registerServiceForDoc(SERVICE_TAG, () => geoDeferred.promise);
AMP.registerServiceForDoc(SERVICE_TAG, function() {
return geoDeferred.promise;
});
});
4 changes: 3 additions & 1 deletion extensions/amp-next-page/0.1/amp-next-page.js
Original file line number Diff line number Diff line change
Expand Up @@ -302,6 +302,8 @@ function extractAdSenseTextContent(el) {

AMP.extension(TAG, '0.1', AMP => {
const service = new NextPageService();
AMP.registerServiceForDoc(SERVICE_ID, () => service);
AMP.registerServiceForDoc(SERVICE_ID, function() {
return service;
});
AMP.registerElement(TAG, AmpNextPage, CSS);
});
Original file line number Diff line number Diff line change
Expand Up @@ -622,7 +622,7 @@ AMP.extension(TAG, '0.1', function(AMP) {
* @param {!../../../src/service/ampdoc-impl.AmpDoc} ampdoc
* @return {*} TODO(#23582): Specify return type
*/
ampdoc => {
function(ampdoc) {
const platformService = new GoogleSubscriptionsPlatformService(ampdoc);
const element = ampdoc.getHeadNode();
Services.subscriptionsServiceForDoc(element).then(service => {
Expand Down

0 comments on commit 09d4985

Please sign in to comment.