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

Destructuring an object with a string computed property throws an error #3145

Closed
trueadm opened this issue Nov 14, 2018 · 8 comments
Closed
Labels
P2 triage-done Has been reviewed by someone on triage rotation.

Comments

@trueadm
Copy link

trueadm commented Nov 14, 2018

Given the current test case:

// ==ClosureCompiler==
// @output_file_name default.js
// @compilation_level ADVANCED_OPTIMIZATIONS
// @language_in ECMASCRIPT6_STRICT
// @language_out ECMASCRIPT6_STRICT
// ==/ClosureCompiler==

var React = require("react");

var {
  ["useState"]: useState
} = React;

function NameComponent() {
  const [name] = useState("Ben");
  return React.createElement("div", null, name);
}

module.exports = NameComponent;

The following error is thrown:

23: java.lang.UnsupportedOperationException: COMPUTED_PROP 4 [length: 22] [source_file: Input_0] is not a string node
	at com.google.javascript.rhino.Node.getString(Node.java:1226)
	at com.google.javascript.jscomp.AnalyzePrototypeProperties$ProcessProperties.visit(AnalyzePrototypeProperties.java:324)
	at com.google.javascript.jscomp.NodeTraversal.traverseBranch(NodeTraversal.java:855)
	at com.google.javascript.jscomp.NodeTraversal.traverseChildren(NodeTraversal.java:967)
	at com.google.javascript.jscomp.NodeTraversal.traverseBranch(NodeTraversal.java:851)
	at com.google.javascript.jscomp.NodeTraversal.traverseChildren(NodeTraversal.java:967)
	at com.google.javascript.jscomp.NodeTraversal.traverseBranch(NodeTraversal.java:851)
	at com.google.javascript.jscomp.NodeTraversal.traverseChildren(NodeTraversal.java:967)
	at com.google.javascript.jscomp.NodeTraversal.handleScript(NodeTraversal.java:805)
	at com.google.javascript.jscomp.NodeTraversal.traverseBranch(NodeTraversal.java:830)
	at com.google.javascript.jscomp.NodeTraversal.traverseChildren(NodeTraversal.java:967)
	at com.google.javascript.jscomp.NodeTraversal.traverseBranch(NodeTraversal.java:851)
	at com.google.javascript.jscomp.NodeTraversal.traverse(NodeTraversal.java:336)
	at com.google.javascript.jscomp.NodeTraversal.traverse(NodeTraversal.java:346)
	at com.google.javascript.jscomp.AnalyzePrototypeProperties.process(AnalyzePrototypeProperties.java:158)
	at com.google.javascript.jscomp.CrossChunkMethodMotion.process(CrossChunkMethodMotion.java:92)
	at com.google.javascript.jscomp.PhaseOptimizer$NamedPass.process(PhaseOptimizer.java:310)
	at com.google.javascript.jscomp.PhaseOptimizer$Loop.process(PhaseOptimizer.java:455)
	at com.google.javascript.jscomp.PhaseOptimizer.process(PhaseOptimizer.java:231)
	at com.google.javascript.jscomp.Compiler.performOptimizations(Compiler.java:2459)
	at com.google.javascript.jscomp.Compiler$3.call(Compiler.java:843)
	at com.google.javascript.jscomp.Compiler$3.call(Compiler.java:839)
	at com.google.javascript.jscomp.CompilerExecutor.runInCompilerThread(CompilerExecutor.java:129)
	at com.google.javascript.jscomp.Compiler.runInCompilerThread(Compiler.java:871)
	at com.google.javascript.jscomp.Compiler.stage2Passes(Compiler.java:838)
	at com.google.javascript.jscomp.Compiler.compile(Compiler.java:734)
	at com.google.javascript.jscomp.webservice.backend.CompilerInvokerImpl.compile(CompilerInvokerImpl.java:46)
	at com.google.javascript.jscomp.webservice.backend.ServerController.executeRequest(ServerController.java:181)
	at com.google.javascript.jscomp.webservice.backend.CompilationRequestHandler.serviceParsedRequest(CompilationRequestHandler.java:180)
	at com.google.javascript.jscomp.webservice.backend.CompilationRequestHandler.service(CompilationRequestHandler.java:162)
	at com.google.javascript.jscomp.webservice.frontend.CompilationServlet.doPost(CompilationServlet.java:85)
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:707)
	at javax.servlet.http.HttpServlet.service(HttpServlet.java:790)
	at org.eclipse.jetty.servlet.ServletHolder.handle(ServletHolder.java:848)
	at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1772)
	at com.google.apphosting.utils.servlet.JdbcMySqlConnectionCleanupFilter.doFilter(JdbcMySqlConnectionCleanupFilter.java:60)
	at org.eclipse.jetty.servlet.ServletHandler$CachedChain.doFilter(ServletHandler.java:1759)
	at org.eclipse.jetty.servlet.ServletHandler.doHandle(ServletHandler.java:582)
	at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:143)
	at org.eclipse.jetty.security.SecurityHandler.handle(SecurityHandler.java:524)
	at org.eclipse.jetty.server.session.SessionHandler.doHandle(SessionHandler.java:226)
	at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:143)
	at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:134)
	at com.google.apphosting.runtime.jetty9.ParseBlobUploadHandler.handle(ParseBlobUploadHandler.java:119)
	at org.eclipse.jetty.server.handler.ContextHandler.doHandle(ContextHandler.java:1182)
	at com.google.apphosting.runtime.jetty9.AppEngineWebAppContext.doHandle(AppEngineWebAppContext.java:171)
	at org.eclipse.jetty.servlet.ServletHandler.doScope(ServletHandler.java:512)
	at org.eclipse.jetty.server.session.SessionHandler.doScope(SessionHandler.java:185)
	at org.eclipse.jetty.server.handler.ContextHandler.doScope(ContextHandler.java:1112)
	at org.eclipse.jetty.server.handler.ScopedHandler.handle(ScopedHandler.java:141)
	at com.google.apphosting.runtime.jetty9.AppVersionHandlerMap.handle(AppVersionHandlerMap.java:296)
	at org.eclipse.jetty.server.handler.HandlerWrapper.handle(HandlerWrapper.java:134)
	at org.eclipse.jetty.server.Server.handle(Server.java:539)
	at org.eclipse.jetty.server.HttpChannel.handle(HttpChannel.java:333)
	at com.google.apphosting.runtime.jetty9.RpcConnection.handle(RpcConnection.java:202)
	at com.google.apphosting.runtime.jetty9.RpcConnector.serviceRequest(RpcConnector.java:81)
	at com.google.apphosting.runtime.jetty9.JettyServletEngineAdapter.serviceRequest(JettyServletEngineAdapter.java:123)
	at com.google.apphosting.runtime.JavaRuntime$RequestRunnable.dispatchServletRequest(JavaRuntime.java:699)
	at com.google.apphosting.runtime.JavaRuntime$RequestRunnable.dispatchRequest(JavaRuntime.java:661)
	at com.google.apphosting.runtime.JavaRuntime$RequestRunnable.run(JavaRuntime.java:631)
	at com.google.apphosting.runtime.JavaRuntime$NullSandboxRequestRunnable.run(JavaRuntime.java:825)
	at com.google.apphosting.runtime.ThreadGroupPool$PoolEntry.run(ThreadGroupPool.java:273)
	at java.lang.Thread.run(Thread.java:745)

Original Post Data: 
output_format=json&output_info=compiled_code&output_info=warnings&output_info=errors&output_info=statistics&compilation_level=ADVANCED_OPTIMIZATIONS&warning_level=verbose&language_out=ECMASCRIPT6_STRICT&output_file_name=default.js&js_code=var%20React%20%3D%20require(%22react%22)%3B%0A%0Avar%20%7B%0A%20%20%5B%22useState%22%5D%3A%20useState%0A%7D%20%3D%20React%3B%0A%0Afunction%20Component_ComputeFunction()%20%7B%0A%20%20const%20%5Bname%5D%20%3D%20useState(%22Ben%22)%3B%0A%20%20return%20React.createElement(%22div%22%2C%20null%2C%20name)%3B%0A%7D%0A%0Avar%20Component%20%3D%20%2F%2F%20Component%20OPCODES%0A%5B1%20%2F%2F%20COMPONENT_WITH_HOOKS%0A%2C%202%20%2F%2F%20DISPLAY_NAME%0A%2C%20%22Component%22%2C%203%20%2F%2F%20ROOT_PROPS_SHAPE%0A%2C%20null%2C%2020%20%2F%2F%20UNCONDITIONAL_TEMPLATE%0A%2C%20%5B7%20%2F%2F%20OPEN_ELEMENT_DIV%0A%2C%2042%20%2F%2F%20ELEMENT_DYNAMIC_TEXT_CHILD%0A%2C%200%2C%2042%20%2F%2F%20ELEMENT_DYNAMIC_TEXT_CHILD%0A%2C%201%2C%209%20%2F%2F%20CLOSE_ELEMENT%0A%5D%2C%20Component_ComputeFunction%5D%3B%0Amodule%5B%22exports%22%5D%20%3D%20Component%3B
@prateekbh
Copy link

i am facing something similar

DESTRUCTURING_LHS 23 [length: 20] [source_file: src/polyfills/object-assign.js] is not a string node
  Node(CONST): src/polyfills/object-assign.js:23:0
const {prototype} = Object;
  Parent(SCRIPT): src/polyfills/object-assign.js:17:0
const Object = {

        at com.google.javascript.rhino.Node.getString(Node.java:1113)
        at org.ampproject.AmpPass.maybeReplaceRValueInVar(Unknown Source)
        at org.ampproject.AmpPass.visit(Unknown Source)
        at com.google.javascript.jscomp.NodeTraversal.traverseBranch(NodeTraversal.java:769)
        at com.google.javascript.jscomp.NodeTraversal.traverseChildren(NodeTraversal.java:840)
        at com.google.javascript.jscomp.NodeTraversal.handleScript(NodeTraversal.java:721)
        at com.google.javascript.jscomp.NodeTraversal.traverseBranch(NodeTraversal.java:746)
        at com.google.javascript.jscomp.NodeTraversal.traverseChildren(NodeTraversal.java:840)
        at com.google.javascript.jscomp.NodeTraversal.traverseBranch(NodeTraversal.java:765)
        at com.google.javascript.jscomp.NodeTraversal.traverse(NodeTraversal.java:305)
        at com.google.javascript.jscomp.NodeTraversal.traverseEs6(NodeTraversal.java:680)
        at org.ampproject.AmpPass.hotSwapScript(Unknown Source)
        at org.ampproject.AmpPass.process(Unknown Source)
        at com.google.javascript.jscomp.Compiler.process(Compiler.java:1083)
        at com.google.javascript.jscomp.Compiler.runCustomPasses(Compiler.java:1116)
        at com.google.javascript.jscomp.Compiler.check(Compiler.java:1072)
        at com.google.javascript.jscomp.Compiler.performChecksAndTranspilation(Compiler.java:866)
        at com.google.javascript.jscomp.Compiler.access$000(Compiler.java:102)
        at com.google.javascript.jscomp.Compiler$2.call(Compiler.java:800)
        at com.google.javascript.jscomp.Compiler$2.call(Compiler.java:797)
        at com.google.javascript.jscomp.CompilerExecutor$2.call(CompilerExecutor.java:101)
        at java.util.concurrent.FutureTask.run(FutureTask.java:266)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
        at java.lang.Thread.run(Thread.java:748)
Caused by: java.lang.UnsupportedOperationException: DESTRUCTURING_LHS 23 [length: 20] [source_file: src/polyfills/object-assign.js] is not a string node
        ... 25 more

for code like:
const {hasOwnProperty} = Object.prototype;
code link: https://github.com/ampproject/amphtml/blob/master/src/polyfills/object-assign.js#L17

@brad4d
Copy link
Contributor

brad4d commented Nov 16, 2018

Created internal Google issue b/119638231

@brad4d brad4d added the triage-done Has been reviewed by someone on triage rotation. label Nov 16, 2018
@brad4d
Copy link
Contributor

brad4d commented Nov 16, 2018

@prateekbh in your case the failure is happening in code that is not part of closure-compiler.
AmpPass needs to be updated to understand how to handle destructuring assignments.

If this failure has only recently started happening, that is because we recently moved the transpilation of destructuring assignments later in the compilation process. Probably it previously happened before the exection of AmpPass, so it never saw any destructuring code, but has now been moved after.

@brad4d
Copy link
Contributor

brad4d commented Nov 16, 2018

@trueadm it looks like we need to update AnalyzePrototypeProperties to make sure it understands computed properties.

We haven't done that yet, because in the closure-compiler code it is only used within CrossChunkCodeMotion, which is a pass that runs after all language features above ES5 have been removed from the code. Apparently AmpPass also uses it. See my comment above.

@brad4d
Copy link
Contributor

brad4d commented Nov 16, 2018

We will be fixing AnalyzePrototypeProperties in the next quarter, because we need to make CrossChunkCodeMotion work for language output > ES5, but we can't do anything about the AmpPass.

@brad4d brad4d added the P2 label Nov 16, 2018
@brad4d
Copy link
Contributor

brad4d commented Nov 16, 2018

@trueadm correction: I somehow thought I was looking at your stack trace when I was still looking at the one from @prateekbh

I see your failure is in CrossChunkCodeMotion.
That pass should have already been updated to handle destructuring.
Looks like we failed to notice the issue with AnalyzePrototypeProperties when we enabled CrossChunkCodeMotion for language output levels > ES5.

We'll fix AnalyzePrototypeProperties.

@lauraharker
Copy link
Contributor

@trueadm I've submitted a fix for the crash internally. It will be pushed to GitHub tomorrow.

@trueadm
Copy link
Author

trueadm commented Nov 20, 2018

@lauraharker Thank you :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P2 triage-done Has been reviewed by someone on triage rotation.
Projects
None yet
Development

No branches or pull requests

4 participants