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

chore: handle ambiguous instances of helpers #350

Closed
wants to merge 1 commit into from

Conversation

colenso
Copy link
Contributor

@colenso colenso commented Jan 8, 2024

This is needed to turn on staticHelpers embroider flag.

@colenso
Copy link
Contributor Author

colenso commented Jan 8, 2024

Trying to figure out why the CI is breaking here and what to do about it. Getting some help in the Embroider discord community @mkszepp not sure if you're on discord but here's the thread.

@mkszepp
Copy link
Collaborator

mkszepp commented Jan 8, 2024

@colenso Yes, I also have Discrod. Unfortunately this channel is not public or something. This is result, when i'm logged in. Maybe i need a invite?
grafik

@mkszepp
Copy link
Collaborator

mkszepp commented Jan 8, 2024

Note: i'm using in my apps all embroider settings with true also staticHelpers: true, and it works without any issues

@colenso
Copy link
Contributor Author

colenso commented Jan 8, 2024

@mkszepp What's your discord username?

@mkszepp
Copy link
Collaborator

mkszepp commented Jan 8, 2024

@colenso exor007 is my discord username

@mkszepp
Copy link
Collaborator

mkszepp commented Jan 9, 2024

@colenso i'm closing this PR, because its not necessary... your changes are working only in embroider apps, but we want to hold compatiblity with classic build.

Summary of Discord communication:

For embroider apps, there is always necessary to add all of this option with true or false. This is because embroider wants that (see https://github.com/embroider-build/embroider/blob/a74ae6955933434d335bc9475299cd739437f7a6/docs/upgrade-guides.md?plain=1#L43-L47)

staticHelpers: true,
staticModifiers: true,
staticComponents: true,

The test embroider-optimized has all this options by default true... so we are sure that everything works, when that test szenario is passing all tests

@mkszepp mkszepp closed this Jan 9, 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 this pull request may close these issues.

3 participants