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

invalid block directive in minify-dead-code-elimination #601

Closed
postspectacular opened this issue Jun 23, 2017 · 6 comments
Closed

invalid block directive in minify-dead-code-elimination #601

postspectacular opened this issue Jun 23, 2017 · 6 comments
Labels
bug Confirmed bug

Comments

@postspectacular
Copy link
Contributor

Just encountered this error (w/ babel-plugin-minify-dead-code-elimination@0.1.7):

TypeError: bundle.js: block.get(...).filter is not a function
    at getUseStrictDirectives (.../node_modules/babel-plugin-minify-dead-code-elimination/lib/remove-use-strict.js:50:34)
    at removeUseStrict (.../node_modules/babel-plugin-minify-dead-code-elimination/lib/remove-use-strict.js:21:20)
    at PluginPass.exit (.../node_modules/babel-plugin-minify-dead-code-elimination/lib/index.js:844:13)
    at newFn (.../node_modules/babel-traverse/lib/visitors.js:276:21)
    at NodePath._call (.../node_modules/babel-traverse/lib/path/context.js:76:18)
    at NodePath.call (.../node_modules/babel-traverse/lib/path/context.js:48:17)
    at NodePath.visit (.../node_modules/babel-traverse/lib/path/context.js:117:8)
    at TraversalContext.visitQueue (.../node_modules/babel-traverse/lib/context.js:150:16)
    at TraversalContext.visitSingle (.../node_modules/babel-traverse/lib/context.js:108:19)
    at TraversalContext.visit (.../node_modules/babel-traverse/lib/context.js:192:19)
    at Function.traverse.node (.../node_modules/babel-traverse/lib/index.js:114:17)

I've added some debug traces and it seems that block.get("directives") doesn't always return an array, but sometimes an object too, in which case this error is triggered. Not sure about the internals of blocks, but maybe that's an upstream issue?

As a quick fix this works:

function getUseStrictDirectives(block) {
    var dir = block.get("directives");
    return dir.filter ? dir.filter(function (directive) {
        return directive.node.value.value === useStrict;
    }) : [];
}
@postspectacular postspectacular changed the title undefined block directive in minify-dead-code-elimination invalid block directive in minify-dead-code-elimination Jun 23, 2017
@vigneshshanmugam
Copy link
Member

Just wondering about the corner case, Can you give the test code where it got failed?

@boopathi boopathi added the bug Confirmed bug label Jun 23, 2017
@boopathi
Copy link
Member

Looks like a bug. Would you like to send a PR?

@postspectacular
Copy link
Contributor Author

@vigneshshanmugam not easily, code base is >40k LOC and couldn't yet determine which part triggers it. I've noticed though that the block in question has an empty body, so guess this is caused by a previous optimization step (path.remove())...

{ type: 'BlockStatement',
  body: [],
  trailingComments: [],
  leadingComments: [],
  innerComments: [] }

So proposing a better fix based on that finding:

function getUseStrictDirectives(block) {
    return block.body && block.body.length ?
        block.get("directives").filter(function (directive) {
            return directive.node.value.value === useStrict;
        }) : [];
}

@boopathi sure, should this be based on master?

@vigneshshanmugam
Copy link
Member

@postspectacular Thanks for explaining. Yes based on master.

@postspectacular
Copy link
Contributor Author

@vigneshshanmugam no prob, still i think the real issue is somewhere else (in babel?) which causes block.get("directives") to return wrong data (NodePath instead of array) - this current fix is just addressing symptoms, not a cure...

@IgnusG
Copy link

IgnusG commented Jul 17, 2017

I'm still getting this error. Running babel-preset-babili 0.1.4 with babel-cli 6.24.1 on node 8.1.3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Confirmed bug
Projects
None yet
Development

No branches or pull requests

4 participants