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

Incomplete tree shaking (cjs?) #898

Closed
Benjamin-Dobell opened this issue Sep 5, 2016 · 4 comments
Closed

Incomplete tree shaking (cjs?) #898

Benjamin-Dobell opened this issue Sep 5, 2016 · 4 comments

Comments

@Benjamin-Dobell
Copy link
Contributor

Benjamin-Dobell commented Sep 5, 2016

Version

"rollup version 0.34.13"

Details

I have an ES6 module ("dom") that looks like this:


// ... code ...

var isDescendant = function(parent, child) {
    // ... code ...
};

// ... code ...

export {
    addEventListener,
    getElementContentWidth,
    getElementContentHeight,
    removeAllChildren,
    isDescendant,
    loadCss
};

The tree shaking is detecting that isDescendant is not called, and hence:

var isDescendant = function(parent, child) {
    // ... code ...
};

is completely removed from my target CJS bundle.

However, the "transpiled" export referring to isDescendent is left in place:

var dom = Object.freeze({
    addEventListener: addEventListener,
    getElementContentWidth: getElementContentWidth,
    getElementContentHeight: getElementContentHeight,
    removeAllChildren: removeAllChildren,
    isDescendant: isDescendant,
    loadCss: loadCss
});

Which results in "ReferenceError: isDescendant is not defined".

@TrySound
Copy link
Member

TrySound commented Sep 5, 2016

@Benjamin-Dobell Like this?
example

@Benjamin-Dobell
Copy link
Contributor Author

Benjamin-Dobell commented Sep 5, 2016

@TrySound I've worked out a minimum reproduction case. The indirection (namespace-like behaviour) seems to cause the issue:

example

Rich-Harris added a commit that referenced this issue Sep 6, 2016
@Rich-Harris
Copy link
Contributor

Thanks, I've added that reproduction as a test case in #902, where it passes (rather than including the unusedFunc declaration, it knows that it doesn't need to reify the unused namespace in the first place – in other words, all the code disappears which is a better result)

@Rich-Harris
Copy link
Contributor

Fixed in 0.35

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

No branches or pull requests

3 participants