Fix issue with unused import being emitted

[mewhhaha]

Thanks for contributing to react-virtualized!

Before submitting a pull request, please complete the following checklist:

• For any new features or bug fixes, both positive and negative test cases have been added
• For any new features, documentation has been added
• For any documentation changes, the text has been proofread and is clear to both experienced users and beginners.
• The existing test suites (npm test) all pass
• Format your code with prettier (yarn run prettier).
• Run the Flow typechecks (yarn run typecheck).

Description

brigand/babel-plugin-flow-react-proptypes#206
#1632
#1212

There seems to be an issue with an unused import being emitted in the onScroll file as a result of the babel-plugin-flow-react-proptypes plugin. The unused import makes it unable to build with esbuild, and in turn Vite and Snowpack has issues. The suggested fix is to add the supression string as described in the "Suppression" section of https://github.com/brigand/babel-plugin-flow-react-proptypes at the top of the file. This should not be an issue as there are no React proptypes to be generated in this file.

What do you think? Thanks in beforehand. With friendly hellos.

[johnpangalos]

Hey, is there anyone who is a contributor/maintainer of this project that could look at this? I would really appreciate this change going through. It's one line of code (almost a comment but the underlying lib doesn't support that) and it's recommended by the maintainer of babel-plugin-flow-react-proptypes (the lib causing the errors that react-virtualized uses) to fix such issues.

Thanks for all the great work and sorry for being pushy.

[bobbyrenwick]

@mewhhaha @johnpangalos and anyone else who found themselves stuck in their esbuild transition here.

I have worked around the issue with a plugin, add a file next to your esbuild script called esbuild.plugin-fix-react-virtualized.js

const plugin = () => ({
    name: "fix-react-virtualized-plugin",
    setup(build) {
        build.onLoad(
            {
                filter: /react-virtualized\/dist\/es\/WindowScroller\/utils\/onScroll.js$/,
            },
            async args => {
                let text = await fs.promises.readFile(args.path, "utf8");

                return {
                    contents: text.replace(
                        'import { bpfrpt_proptype_WindowScroller } from "../WindowScroller.js";',
                        ""
                    ),
                };
            }
        );
    },
});

module.exports.plugin = plugin;

and then in your build script:

const fixReactVirtualized = require("./esbuild.plugin-fix-react-virtualized");

esbuild.build({
    // ...
    plugins: [
        // ...
        fixReactVirtualized.plugin(),
    ],
    // ...
}).catch(() => {
    process.exit(1);
});

As long as there's something sensible to replace the broken text with you can apply this same plugin fix to lots of libs. I've had to do the same thing for a dependency of react-dates in order to get that to work too.

[huyng12]

I use Vite (which uses rollup) and facing with this issue too. Could you guys tell me how to fix? I'm stuck with this issue for a week 😢 @bobbyrenwick

[bobbyrenwick]

@huyng12 - you should be able to use the logic above but within the Vite Plugin API - https://vitejs.dev/guide/api-plugin.html#transforming-custom-file-types

[bobbyrenwick]

@huyng12 I might be wrong actually as I think that Vite actually uses esbuild to build your dependencies so the rollup plugins won't be applied to the files of your dependencies.

Perhaps you could use react-window instead of react-virtualized though I'm not 100% sure that would solve the issue but if you imported the similar components into your file and then you didn't get any errors at least you'd know that it'd be worth the time investment to switch to the different API.

Or you could switch from vite to esbuild instead and use the plugin above. We recently went from webpack -> snowpack (though I tried vite and failed to get it to run) and quickly reverted to esbuild building everything instead. Snowpack is pretty similar to vite and we had some issues with the no bundler approach.

[huyng12]

Thank you @bobbyrenwick, I worked around by remove that import in that package in node_modules then use patch-package for applying that fix for the later use. I found it related to #1632.

[bobbyrenwick]

@huyng12 glad that you got it sorted in the end! I've never heard of patch-package so thanks for the heads up. It looks very useful!

[memark]

What is the status on this PR? Would love to have this fix merged.

[hsblhsn]

Is there any update about this PR?

[wuweiweiwu]

Thanks for this change! will notify this thread when it is released

[benatshippabo]

unrelated, but @wuweiweiwu any chance you could review this? frontend-collective/frontend-collective-react-dnd-scrollzone#80 it's blocking several people from upgrading to react 17

[0rvar]

@wuweiweiwu any plans to cut a new release with this?

[ruslanchek]

Looks abandoned, like most of npm modules :-(
PR Merged but not released since September.

[0rvar]

We're actively moving away from react-virtualized since it seems abandoned. Moving to an in-house hook based solution that is better suited for modern react with hook-based memoization

[memark]

@bvaughn Any release coming up?

[ruslanchek]

We're actively moving away from react-virtualized since it seems abandoned. Moving to an in-house hook based solution that is better suited for modern react with hook-based memoization

What is the solution?

[johnpangalos]

You could try this https://react-virtual.tanstack.com/docs/overview if it fits your use case.

Making something similar might be a lot of work if you consider the edge cases, here is react-virtual's implementation https://github.com/tannerlinsley/react-virtual/blob/master/src/index.js.

What we've done is just fork this repo and used the repo as the version in our dependency in our package.json.

• https://docs.npmjs.com/cli/v7/configuring-npm/package-json#git-urls-as-dependencies
• https://docs.npmjs.com/cli/v7/configuring-npm/package-json#github-urls

None of which is ideal, but I hope it helps.

[charles-stuart-appdetex]

Another solution by way of your package.json:

"postinstall": "npx replace-in-files-cli --string='import { bpfrpt_proptype_WindowScroller } from \"../WindowScroller.js\";' --replacement='' node_modules/**/onScroll.js",

You will need to add replace-in-files-cli to your devDependencies.

[jasonwilliams]

Hey @bvaughn is there any reason you're not doing any releases?

Between #1212 and #1632 this issue has had over 60 upvotes. It's quite important to people suffering from this problem. It looks like you're still maintaining this project so I'm confused at there being no release for 2 years.

[ghost]

This has finally landed in https://github.com/bvaughn/react-virtualized/releases/tag/9.22.4 (and 9.22.5 for which I can't find the code?!) but the import is still there, so the issue still exists.

[bvaughn]

Hey @bvaughn is there any reason you're not doing any releases?

Yeah. This is too much to keep up with.

[LvChengbin]

Are there any plans of fixing this issue?

[testn]

I still see the issue.