Skip to content

RFC: add Module.includeAllInBundleRecursive - #1887

Closed
kellyselden wants to merge 2 commits into
rollup:masterfrom
kellyselden:includeAllInBundleRecursive
Closed

RFC: add Module.includeAllInBundleRecursive#1887
kellyselden wants to merge 2 commits into
rollup:masterfrom
kellyselden:includeAllInBundleRecursive

Conversation

@kellyselden

Copy link
Copy Markdown
Contributor

Uses #1845 as a base to create meaningful tests. That description also perfectly applies here #1845 (comment).

@guybedford

Copy link
Copy Markdown
Contributor

I'm not sure I follow the use case here, is there an issue tracking the feature or further description somewhere?

@kellyselden

Copy link
Copy Markdown
Contributor Author

Expanding upon #1845 (comment), I essentially want to add the module with the missing export to the entry modules. This PR has to be rewritten for the new system, but the use case still remains. I wish this use case didn't have to exist, but it is a interop concern that will hopefully correct itself once we roll this out to Ember users with a warning. Here are a couple of examples I found with my testing:
DockYard/ember-composable-helpers#287
jmurphyau/ember-truth-helpers#78
https://github.com/samselikoff/ember-cli-mirage/blob/v0.4.1/app/initializers/ember-cli-mirage.js#L4 testConfig doesn't exist here https://github.com/samselikoff/ember-cli-mirage/blob/v0.4.1/blueprints/ember-cli-mirage/files/__root__/config.js

@guybedford

Copy link
Copy Markdown
Contributor

Does the interop option not make the default export grabbing work through Rollup?

So if I'm hearing you right, this change is necessary to ensure the missing export hook triggers correctly?

I've very concerned that the missing export hook merged exposes the internal Module API. This isn't a public API we should be exposing, and I think we should change that before it gets relied on.

//cc @lukastaegert

@guybedford

Copy link
Copy Markdown
Contributor

Also it would help to hear what the exact fix is that you are implementing through the missing export hook.

@kellyselden

Copy link
Copy Markdown
Contributor Author

The pseudo code is

missingExport(entryPoints, module) {
  // print deprecation
  entryPoints.push(module);
}

As you can see, I don't need any Module API after the chunking change, but I still need a way to continue traversing a broken tree.

@guybedford

Copy link
Copy Markdown
Contributor

Is this PR still valid?

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