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(rollup-build): Modified rollup config #2

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

kmorope
Copy link

@kmorope kmorope commented Feb 29, 2024

Copy link
Owner

@vselvam17 vselvam17 left a comment

Choose a reason for hiding this comment

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

This is a sample code and not the exact source code what we have in our production. But this is a just a mimic to see the twilio issue.

Rollup configurations are just boiler plate code and we don't use rollup to build the components, and we use only tsc build. FYI, Our module system uses ES pattern and not common JS, so we are restricted to use commonjs dependencies.

The build output is very key for us because this component is integrated with multiple applications which refers different files as an entry point to initialise the component.

Just to give an example of the build output, PFA

Screenshot 2024-03-12 at 12 42 50

We are not using any minified version of the build output, this build bundle would be used to integrate to the application. the application bundle is minified.

For roll up bundling, ING has their own proprietary tool kit to use.

So kindly ignore rollup to build the component, if the issue could only be resolved by using a rollup configuration, then we expect a similar build output and not the minified version and by only using the ESM dependencies.

Also, you can ignore the bundled.js file import in HTML which is not recommended within ING

Expectation:

  1. We need the same build output as mentioned in the screenshot without using rollup and relying only on ts configurations.
  2. Even if using rollup configuration it should have the same build output pattern and not a minified file and also not using CommonJS dependencies

Feel free to ask more queries in case if you have any.

I will also try to update this code a bit more to be aligned with the production code.

@vselvam17
Copy link
Owner

Removed unwanted code in the feature.

  1. Removed rollup configuration
  2. index.html solely used for demo purpose and not advisable to include a bundled output file, same case considering the production code.

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