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

Fix module entry point by migrating to vite for ESM building #4276

Merged
merged 3 commits into from
Jul 12, 2023

Conversation

susnux
Copy link
Contributor

@susnux susnux commented Jun 26, 2023

☑️ Resolves

This introduces vite for building the module entry point instead of webpack.

  1. The previously generated module by webpack is invalid as it still uses require instead of import which is not defined for ES modules.
  2. Build time is much shorter
  3. The resulting file is only 600 kiB instead of 930 kiB

(Different from #3529 as this just introduces vite for module building to fix the assets)

🏁 Checklist

  • ⛑️ Tests are included or are not applicable
  • 📘 Component documentation has been extended, updated or is not applicable

@susnux susnux added bug Something isn't working 3. to review Waiting for reviews configuration Pull requests that update a config file labels Jun 26, 2023
@susnux susnux force-pushed the feat/migrate-to-vite branch from 649b09e to 593eb34 Compare June 26, 2023 18:07
@ShGKme
Copy link
Contributor

ShGKme commented Jun 27, 2023

Cannot we make a full migration to Vite?
Or fix the problem with Webpack config, if possible.

I'm a bit afraid of having 2 (almost 3) different bundlers at the same time in terms of config support.

@skjnldsv
Copy link
Contributor

Cannot we make a full migration to Vite? Or fix the problem with Webpack config, if possible.

I'm a bit afraid of having 2 (almost 3) different bundlers at the same time in terms of config support.

I think the goal is to do a step-by-step migration :)

@susnux
Copy link
Contributor Author

susnux commented Jun 27, 2023

Cannot we make a full migration to Vite?

That was / is my plan, but currently it does not work out that good because styleguide does not work that well with vite currently (see #2959 and #3529 and #3565 ).
So I wanted to first migrate this part to fix the module bundle, and then later migrate the other part.

(edit:)

I think the goal is to do a step-by-step migration :)

Exactly from those three previous PRs I know that doing all in one is quite hard and error prone, so doing it step by step seems to be the best way :)

@susnux susnux force-pushed the feat/migrate-to-vite branch from 2b3b728 to 63b6ede Compare July 2, 2023 17:32
@susnux susnux requested a review from a team July 2, 2023 17:37
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
cypress.config.mjs Outdated Show resolved Hide resolved
@susnux susnux force-pushed the feat/migrate-to-vite branch 2 times, most recently from 715e67f to 78c6026 Compare July 4, 2023 12:19
vite.config.mts Outdated Show resolved Hide resolved
@susnux susnux force-pushed the feat/migrate-to-vite branch 2 times, most recently from 178fc7e to c7a5273 Compare July 4, 2023 13:02
@susnux susnux requested a review from ShGKme July 10, 2023 06:15
.eslintrc.js Outdated Show resolved Hide resolved
@susnux susnux force-pushed the feat/migrate-to-vite branch from c7a5273 to 96663e6 Compare July 11, 2023 15:47
@susnux susnux requested a review from juliusknorr July 11, 2023 15:48
Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
susnux added 2 commits July 11, 2023 18:02
* Webpack emitted invalid ES module code, as it still contained `require` calls which
  are undefined on ES modules
* Build time is much shorter
* Built assets are much smaller (570 kB vs 930 kB)
* Remove unneeded import

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
…pack and cypress

When imported in vite config it will be executed in ESM context where dynamic require of modules is not available.
But when converted to ES module it works in vite config + webpack config, but not in Cypress config.
Because our package type is `commonjs` so Typescript files are executed as CommonJS Typescript, so the Cypress config can not require "import" an ES module.

Solutions: Either set out package to `type: "module"` or as done here rewrite all require calls to dynamic import which is available on CommonJS *and* ES module.

Signed-off-by: Ferdinand Thiessen <opensource@fthiessen.de>
@susnux susnux force-pushed the feat/migrate-to-vite branch from 96663e6 to eafc329 Compare July 11, 2023 16:02
@skjnldsv skjnldsv merged commit 0e61454 into master Jul 12, 2023
@skjnldsv skjnldsv deleted the feat/migrate-to-vite branch July 12, 2023 05:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews bug Something isn't working configuration Pull requests that update a config file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants