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

ESM output contains many duplicated helpers #2230

Open
rschristian opened this issue Apr 9, 2024 · 3 comments · Fixed by #2278
Open

ESM output contains many duplicated helpers #2230

rschristian opened this issue Apr 9, 2024 · 3 comments · Fixed by #2278

Comments

@rschristian
Copy link

rschristian commented Apr 9, 2024

Description

As @docsearch/react compiles each of its files in isolation for the ESM output, the module as a whole ends up with a dozen duplicated helpers even in the most simple demonstration.

Edit: This applies for all dependencies of @docsearch/react too.

This heavily bloats the bundles of applications which use it, and bloats them needlessly.

Steps to reproduce

  1. Start a Vite (though every bundler would be impacted similarly) React project, pasting the following into src/main.jsx:
import React from 'react';
import ReactDOM from 'react-dom/client';
import { DocSearch } from '@docsearch/react';

function App() {
  return (
    <DocSearch
      appId="YOUR_APP_ID"
      indexName="YOUR_INDEX_NAME"
      apiKey="YOUR_SEARCH_API_KEY"
    />
  );
}

ReactDOM.createRoot(document.getElementById('root')).render(
  <React.StrictMode>
    <App />
  </React.StrictMode>,
)
  1. npm run build
  2. Open built JS
  3. See 10 copies of the iterator helper ("Invalid attempt to destructure non-iterable instance....")

Expected behavior

That there aren't heavily duplicated helpers throughout the bundles.

This likely means moving to better build tooling, as compiling each module in isolation isn't really viable, nor has it ever been. The asset quality takes a pretty big hit when doing so.

Environment

  • OS: N/A
  • Browser: N/A
  • DocSearch version: 3.6.0
@randombeeper randombeeper added the Needs investigation Investigation is planned but not started yet label Jul 11, 2024
@levimichael levimichael added Investigation in progress This issue is being investigated and removed Needs investigation Investigation is planned but not started yet labels Jul 12, 2024
levimichael added a commit that referenced this issue Jul 15, 2024
levimichael added a commit that referenced this issue Jul 15, 2024
levimichael added a commit that referenced this issue Jul 16, 2024
…ypes (#2278)

* fix #2230

* fix linting

* improve docsearch-js build process, update all rollup configs, delete some deps
@levimichael levimichael removed the Investigation in progress This issue is being investigated label Jul 16, 2024
@levimichael
Copy link
Contributor

@rschristian thanks for reporting this. We just released a fix, let me know if it looks good on your end now.

@rschristian
Copy link
Author

Marginally improved, but still a very large amount of duplication.

Looking at the bundle, there's now only 2 copies of the iterator helper ("Invalid attempt to destructure...") down from 10, but 5 copies of the spread helper ("Invalid attempt to spread..."), 14 copies of the toPrimitive helper ("@@toPrimitive must return..."), and 30 copies of the object spread polyfill (Object.getOwnPropertyDescriptors ? ... :).

Modern compression being a marvel means this isn't too egregious over the web but it does still have a large & needless impact on parse times. I wouldn't be surprised if 20%+ of the minified bundle is just duplicated helpers.

@levimichael
Copy link
Contributor

Ah, I was just looking at helpers that were duplicated in the bundling of @docsearch/react itself. It appears that we are importing dependencies that also have these helpers duplicated in their bundles as well. Will require further investigation. Thanks for following up.

@levimichael levimichael reopened this Jul 16, 2024
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

Successfully merging a pull request may close this issue.

3 participants