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 re-exports for helpers #573

Merged
merged 1 commit into from
Mar 19, 2024

Conversation

bertdeblock
Copy link
Contributor

Probably an oversight during the v1 -> v2 conversion?

@machty
Copy link
Owner

machty commented Mar 19, 2024

Hmm how come we don't want this? Don't we still want ember-cli apps to have {{perform ...}} {{cancel-all ...}} and {{task ...}} to be available in pre-.gts components?

@bertdeblock
Copy link
Contributor Author

@machty
Copy link
Owner

machty commented Mar 19, 2024

I'm sure you're correct, and I'm just about to merge this, but for posterity: where does _app_ folder come from? What is the configuration/mechanism that puts these helpers into dist/_app_/?

@bertdeblock
Copy link
Contributor Author

bertdeblock commented Mar 19, 2024

The entries in the package.json file are auto-generated AFAIK, based on https://github.com/machty/ember-concurrency/blob/master/packages/ember-concurrency/rollup.config.mjs#L23.

This is done via a custom Rollup plugin in @embroider/addon-dev.
Code that does the mapping => https://github.com/embroider-build/embroider/blob/main/packages/addon-dev/src/rollup-app-reexports.ts#L33.

The _app_ folder is generated by Embroider, based on the defined app reexports and is the v2-addon equivalent of the app folder in v1 addons.

You can see it in ember-concurrency's dist folder => https://www.npmjs.com/package/ember-concurrency?activeTab=code

Also note that, the app folder that is deleted in this PR currently isn't published => https://github.com/machty/ember-concurrency/blob/master/packages/ember-concurrency/package.json#L63-L68, proving it's redundant.

@machty machty merged commit ce0b142 into machty:master Mar 19, 2024
17 checks passed
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.

None yet

2 participants