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

breaking: re-export configModule to reduce duplication and fix dynamic config #41

Merged
merged 1 commit into from
Apr 22, 2022

Conversation

mansona
Copy link
Owner

@mansona mansona commented Apr 22, 2022

Thanks to @pichfl for the pairing session and deep-diving into ember-cli to get this super neat πŸ‘¨β€πŸ³ πŸ’‹

This is only breaking because if you were relying on dynamic config changes (by altering the config HTML) then before this PR it wasn't working, and now because of this PR it is working πŸŽ‰

Fixes #39
Fixes #40

@mansona mansona changed the title re-export configModule to reduce duplication and fix dynamic config breaking: re-export configModule to reduce duplication and fix dynamic config Apr 22, 2022
@mansona mansona merged commit 25d0a94 into master Apr 22, 2022
@delete-merged-branch delete-merged-branch bot deleted the configModule branch April 22, 2022 16:38
@bertdeblock
Copy link
Contributor

Thanks!

Was just wondering if the dynamic importSync is officially supported or if the import path should be completely constructed in index.js instead.

Similar to this test:
https://github.com/embroider-build/embroider/blob/main/packages/macros/tests/babel/import-sync.test.ts#L49-L55

@bertdeblock
Copy link
Contributor

It seems by using importSync, the config is still included in JS land in an Embroider app.

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.

App config is now included 3 times in final build
2 participants