From 4d8794f9de7d970f361eca660bef86890016fe9c Mon Sep 17 00:00:00 2001 From: aomarks Date: Tue, 17 Jul 2018 15:35:25 -0700 Subject: [PATCH] Handle new Polymer 3 import styles for PolymerElement and the Polymer function in the Closure PolymerPass. Polymer 3 uses ES modules instead of globals. This updates the PolymerPass to recognize identifiers associated with ES module imports (which when the PolymerPass runs are GETPROPs instead of flat NAMEs). NOTE: We're just looking for any GETPROP that ends in the expected identifiers, so this could cause some false positives (i.e. it correctly matches `module$polymer.Polymer`, but also `not.polymer.Polymer`). It seems too brittle to hard-code the module path, though. Open to other approaches. ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=204986835 --- .../jscomp/PolymerPassStaticUtils.java | 38 ++++++++---- .../javascript/jscomp/IntegrationTest.java | 58 +++++++++++++++++++ 2 files changed, 86 insertions(+), 10 deletions(-) diff --git a/src/com/google/javascript/jscomp/PolymerPassStaticUtils.java b/src/com/google/javascript/jscomp/PolymerPassStaticUtils.java index d8dd5219d2e..4b4c42f9353 100644 --- a/src/com/google/javascript/jscomp/PolymerPassStaticUtils.java +++ b/src/com/google/javascript/jscomp/PolymerPassStaticUtils.java @@ -36,19 +36,37 @@ final class PolymerPassStaticUtils { /** @return Whether the call represents a call to the Polymer function. */ @VisibleForTesting - public static boolean isPolymerCall(Node value) { - return value != null && value.isCall() && value.getFirstChild().matchesQualifiedName("Polymer"); + public static boolean isPolymerCall(Node call) { + if (call == null || !call.isCall()) { + return false; + } + Node name = call.getFirstChild(); + // When imported from an ES module, we'll have a GETPROP like + // `module$polymer$polymer_legacy.Polymer`. + return name.matchesQualifiedName("Polymer") + || (name.isGetProp() && name.getLastChild().getString().equals("Polymer")); } - /** @return Whether the class extends Polymer.Element */ + /** @return Whether the class extends PolymerElement. */ @VisibleForTesting - public static boolean isPolymerClass(Node value) { - JSDocInfo info = value == null ? null : NodeUtil.getBestJSDocInfo(value); - return value != null - && value.isClass() - && ((!value.getSecondChild().isEmpty() - && value.getSecondChild().matchesQualifiedName("Polymer.Element")) - || (info != null && info.isPolymer())); + public static boolean isPolymerClass(Node cls) { + if (cls == null || !cls.isClass()) { + return false; + } + // A class with the @polymer annotation is always considered a Polymer element. + JSDocInfo info = NodeUtil.getBestJSDocInfo(cls); + if (info != null && info.isPolymer()) { + return true; + } + Node heritage = cls.getSecondChild(); + // In Polymer 3, the base class was renamed from `Polymer.Element` to `PolymerElement`. When + // imported from an ES module, we'll have a GETPROP like + // `module$polymer$polymer_element.PolymerElement`. + return !heritage.isEmpty() + && (heritage.matchesQualifiedName("Polymer.Element") + || heritage.matchesQualifiedName("PolymerElement") + || (heritage.isGetProp() + && heritage.getLastChild().getString().equals("PolymerElement"))); } /** Switches all "this.$.foo" to "this.$['foo']". */ diff --git a/test/com/google/javascript/jscomp/IntegrationTest.java b/test/com/google/javascript/jscomp/IntegrationTest.java index 234d869d9c1..0ef8ca7d17e 100644 --- a/test/com/google/javascript/jscomp/IntegrationTest.java +++ b/test/com/google/javascript/jscomp/IntegrationTest.java @@ -843,6 +843,64 @@ public void testPolymer2a() { assertThat(compiler.getWarnings()).isEmpty(); } + public void testPolymerElementImportedFromEsModule() { + CompilerOptions options = createCompilerOptions(); + options.setPolymerVersion(2); + options.setWarningLevel(DiagnosticGroups.CHECK_TYPES, CheckLevel.ERROR); + options.declaredGlobalExternsOnWindow = true; + options.setLanguageIn(LanguageMode.ECMASCRIPT_2017); + options.setLanguageOut(LanguageMode.ECMASCRIPT5); + addPolymerExterns(); + + Compiler compiler = + compile( + options, + new String[] { + lines("export class PolymerElement {};"), + lines( + "import {PolymerElement} from './i0.js';", + "class Foo extends PolymerElement {", + " get is() { return 'foo-element'; }", + " static get properties() { return { fooProp: String }; }", + "}", + "const foo = new Foo();", + // This property access would be an unknown property error unless the PolymerPass + // had successfully parsed the element definition. + "foo.fooProp;") + }); + assertThat(compiler.getErrors()).isEmpty(); + assertThat(compiler.getWarnings()).isEmpty(); + } + + public void testPolymerFunctionImportedFromEsModule() { + CompilerOptions options = createCompilerOptions(); + options.setPolymerVersion(2); + options.setWarningLevel(DiagnosticGroups.CHECK_TYPES, CheckLevel.ERROR); + options.declaredGlobalExternsOnWindow = true; + options.setLanguageIn(LanguageMode.ECMASCRIPT_2017); + options.setLanguageOut(LanguageMode.ECMASCRIPT5); + addPolymerExterns(); + + Compiler compiler = + compile( + options, + new String[] { + lines("export function Polymer(def) {};"), + lines( + "import {Polymer} from './i0.js';", + "Polymer({", + " is: 'foo-element',", + " properties: { fooProp: String },", + "});", + // This interface cast and property access would be an error unless the + // PolymerPass had successfully parsed the element definition. + "const foo = /** @type{!FooElementElement} */({});", + "foo.fooProp;") + }); + assertThat(compiler.getErrors()).isEmpty(); + assertThat(compiler.getWarnings()).isEmpty(); + } + public void testPreservedForwardDeclare() { CompilerOptions options = createCompilerOptions(); WarningLevel.VERBOSE.setOptionsForWarningLevel(options);