pqina / filepond

🌊 A flexible and fun JavaScript file upload library
https://pqina.nl/filepond
MIT License
15.25k stars 828 forks source link

Support bundle with treeshaking #381

Open thisconnect opened 5 years ago

thisconnect commented 5 years ago

I just got bitten by that this library needs treeshake disabled https://github.com/pqina/filepond/blob/bbaf944178a8c6a0802f5a0106fb308fbf6269fb/rollup.scripts.js#L41

Rollup seems to treeshake too much away, is this a rollup bug?

The commit where treeshaking was disabled is in https://github.com/pqina/filepond/commit/33878b5cb1269a428259a2399207ed63bc0ccdd6#diff-527d3af57329b4d70ddb9b2ee335f0cb

But it is unclear which code is affected, commit messages mentions "disable tree shaking as it removes placeholder functions"

import * as FilePond from 'filepond'
import FilePondPluginFileValidateSize from 'filepond-plugin-file-validate-size'
import FilePondPluginImageExifOrientation from 'filepond-plugin-image-exif-orientation'
import FilePondPluginImagePreview from 'filepond-plugin-image-preview'

FilePond.registerPlugin(
    FilePondPluginFileValidateSize,
    FilePondPluginImageExifOrientation,
    FilePondPluginImagePreview
)

FilePond.setOptions({
    maxFileSize: '5MB',
    server: 'api/'
})

var pond = FilePond.create(document.querySelector('input[type="file"]'))

Summary

Could you provide more details on why treeshaking needs to be disabled?

Consider opening a rollup issue.

How to reproduce

rollup a bundle that imports filepond

Expected behaviour

the bundle should work, i.e. the example code

Additional information

rollup@1.21.4

Environment Version
OS MacOS
rikschennink commented 5 years ago

I've got these kinds of functions and it tree shaking seems to stop the callbacks from working. Open to suggestions.

const createFoo = () => {
    return {
        onsomething: () => {}
    }
}

// some time later
const foo = createFoo();
foo.onsomething = () => {
     // logic
}
thisconnect commented 5 years ago

@rikschennink that code seems to build fine with rollup, at least with version 1.22

I created a repl https://rollupjs.org/repl/?version=1.22.0&shareable=JTdCJTIybW9kdWxlcyUyMiUzQSU1QiU3QiUyMm5hbWUlMjIlM0ElMjJtYWluLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMmltcG9ydCUyMCU3QmNyZWF0ZUZvbyU3RCUyMGZyb20lMjAnLiUyRm1vZHVsZTEuanMnJTVDbiU1Q24lMkYlMkYlMjBzb21lJTIwdGltZSUyMGxhdGVyJTVDbmNvbnN0JTIwZm9vJTIwJTNEJTIwY3JlYXRlRm9vKCklM0IlNUNuZm9vLm9uc29tZXRoaW5nJTIwJTNEJTIwKCklMjAlM0QlM0UlMjAlN0IlNUNuJTIwJTIwJTIwJTIwJTIwJTJGJTJGJTIwbG9naWMlNUNuJTdEJTVDbiU1Q25leHBvcnQlMjBkZWZhdWx0JTIwZm9vJTIyJTJDJTIyaXNFbnRyeSUyMiUzQXRydWUlN0QlMkMlN0IlMjJuYW1lJTIyJTNBJTIybW9kdWxlMS5qcyUyMiUyQyUyMmNvZGUlMjIlM0ElMjJleHBvcnQlMjBjb25zdCUyMGNyZWF0ZUZvbyUyMCUzRCUyMCgpJTIwJTNEJTNFJTIwJTdCJTVDbiUyMCUyMCUyMCUyMHJldHVybiUyMCU3QiU1Q24lMjAlMjAlMjAlMjAlMjAlMjAlMjAlMjBvbnNvbWV0aGluZyUzQSUyMCgpJTIwJTNEJTNFJTIwJTdCJTdEJTVDbiUyMCUyMCUyMCUyMCU3RCU1Q24lN0QlMjIlN0QlNUQlMkMlMjJvcHRpb25zJTIyJTNBJTdCJTIyZm9ybWF0JTIyJTNBJTIyZXNtJTIyJTJDJTIybmFtZSUyMiUzQSUyMm15QnVuZGxlJTIyJTJDJTIyYW1kJTIyJTNBJTdCJTIyaWQlMjIlM0ElMjIlMjIlN0QlMkMlMjJnbG9iYWxzJTIyJTNBJTdCJTdEJTdEJTJDJTIyZXhhbXBsZSUyMiUzQW51bGwlN0Q=

thisconnect commented 5 years ago

I noticed with treeshaking, that the click bubble animation (like material design) didn't get triggered. Not sure if this is related.

BTW. I don't think you should have to change the code, but rollup should either error or build it.. So I think if you could pin point the issue, I'd suggest to open a rollup issue with it.

rikschennink commented 5 years ago

Alright, I'll test it later this week or at the start of the next

rikschennink commented 5 years ago

@thisconnect I ran some more tests.

The ondragend handler will be set by another function. When treeshaking is enabled, Rollup will remove api.ondragend(position), this will cause FilePond to stop working.

Screenshot 2019-10-08 at 15 20 30

Feel free to open an issue on the Rollup repo (if this is not configurable or I'm missing some configuration thing), unfortunately I'm currently too busy to look into this further.