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

remove Stimulus from bundle #31

Merged

Conversation

adrienpoly
Copy link
Member

relates to : #26

This is a tentative solution to remove Stimulus from the bundle.

It requires that Stimulus is already installed and window.Stimulus is available.
This has been the default for a long time and this code 👇 is very common now a days so I think we should be fine

import { Application } from "@hotwired/stimulus"

const application = Application.start()

// Configure Stimulus development experience
application.debug = false
window.Stimulus = application

export { application }

maybe we can throw an error message if it is not present?

For action cable we don't really have a default object globally available so not sure how to handle this one.

@jorgemanrubia
Copy link
Member

Thanks @adrienpoly 🙏.

I'd prefer a more generic approach here to make sure that no dependencies on the project collide with app dependencies, rather than providing a per-dependency-specific solution. Since this is local development, I find hotwire-spark.js containing everything handy, even if there is duplicated code downloaded. But we certainly don't want any kind of interference with the app's dependencies. My plan was to validate the problem and understand all this better to see how to fix. See #26

Any investigation or exploration you want to make to fix this for good would be welcomed 🙏.

@jorgemanrubia
Copy link
Member

@adrienpoly hey I'll close although I may reopen if we can't find something better here. Thanks again 🙏.

Copy link
Member

@jorgemanrubia jorgemanrubia left a comment

Choose a reason for hiding this comment

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

@adrienpoly after thinking a bit more about this, I think this one is great. Would you mind rebasing 🙏? I'll merge

rollup.config.js Outdated Show resolved Hide resolved
@adrienpoly adrienpoly force-pushed the rollup-external-dependencies branch from 1006b57 to fb7ce15 Compare December 21, 2024 17:05
@adrienpoly
Copy link
Member Author

@jorgemanrubia it is rebased and I removed the rollup config

@jorgemanrubia
Copy link
Member

Thanks @adrienpoly, and sorry for the back and forth 🙏.

@jorgemanrubia jorgemanrubia merged commit 2c3d27e into hotwired:main Dec 21, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants