Skip to content

[LiveComponent] Adding rollup to make an esm build #111

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

Merged
merged 1 commit into from
Jun 29, 2021

Conversation

weaverryan
Copy link
Member

Q A
Bug fix? yes
New feature? no
Tickets Addresses Bug A from #102
License MIT

Hi!

This changes how LiveComponent is bundle. There are a few important pieces:

  1. I decided to continue support IE 11. The difference in the unminified file size (including polyfills) is about 5kg (56kb vs 61kb).

  2. I'm building ESM only right now. I have considered a UMD build... but I don't know if there is a real use-case. In practice, this library will always be used in a build system... but I cannot find any other libraries that only output ESM.

  3. This sets babelHelpers to runtime, which means that instead of embedding the babel helpers (like _typeof

    function _typeof(obj) { "@babel/helpers - typeof"; if (typeof Symbol === "function" && typeof Symbol.iterator === "symbol") { _typeof = function _typeof(obj) { return typeof obj; }; } else { _typeof = function _typeof(obj) { return obj && typeof Symbol === "function" && obj.constructor === Symbol && obj !== Symbol.prototype ? "symbol" : typeof obj; }; } return _typeof(obj); }
    ) in every file, they all import from @babel/runtime.

  4. We're testing the src/ files, not the dist/ files.

I don't have a lot of experience with compiling files for the frontend - and there seems to be a lot of variability on how to do it. If you see anything weird, let me know :).

Cheers!

Comment on lines +13 to +17
external: [
/@babel\/runtime/,
/core-js\//,
'stimulus',
],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the Rollup world we usually automate external by using the following code:

import pkg from './package.json';

export default [
  {
    // ...
    external: Object.keys(pkg.dependencies)
  }
]

WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want to do whatever is standard :). But, the @babel/runtime needed to be a regex so that it matches all the individual, nested paths that are actually imported - like @babel/runtime/.... I got that from the note in this section - https://github.com/rollup/plugins/tree/master/packages/babel#babelhelpers

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh alright, I wasn't aware of that

@@ -1,16 +1,17 @@
{
"name": "@symfony/live-stimulus",
"description": "Live Component: bring server-side re-rendering & model binding to any element.",
"main": "dist/live_controller.js",
"main": "dist/live-controller.esm.js",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think it would be interesting to also configure module field?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though they're identical, yea, I think that's a good idea.

@weaverryan weaverryan merged commit 58e6427 into symfony:main Jun 29, 2021
weaverryan added a commit that referenced this pull request Jul 1, 2021
This PR was merged into the main branch.

Discussion
----------

[bug] remove duplicated doctrine/annotations entry

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| Tickets       | n/a
| License       | MIT

Looks like 2 different PR's added this. (#111 and #106)

Commits
-------

5b9ea2d [bug] remove duplicated doctrine/annotations entry
fullstackdeveloperwebapp added a commit to fullstackdeveloperwebapp/ux that referenced this pull request Aug 1, 2023
This PR was merged into the main branch.

Discussion
----------

[bug] remove duplicated doctrine/annotations entry

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| Tickets       | n/a
| License       | MIT

Looks like 2 different PR's added this. (symfony/ux#111 and symfony/ux#106)

Commits
-------

5b9ea2d [bug] remove duplicated doctrine/annotations entry
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 this pull request may close these issues.

2 participants