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

Fix bug with supporting multiple contextual modules of one type #1222

Merged
merged 3 commits into from Jul 3, 2017

Conversation

justinbmeyer
Copy link
Contributor

@justinbmeyer justinbmeyer commented Jul 2, 2017

This fixes and adds tests for a problem with multiple clones (#1221). The fix is prevent only a single contextual loaded module from being created. Previously, a new contextual module would be defined only if the following check passed:

if(!loader.has(name)) {

As name was always steal-clone, this would only allow one contextual module.

I changed to to:

if(!loader.has(localName)) {

This will only allow one contextual module of that name+parentName combination to be loaded.

Test are not passing for me locally ... I assume this is because I'm on node 5. Because I didn't have bower installed.

@justinbmeyer justinbmeyer changed the title demonstrates error with cloning within dynamic imports. demonstrates error with cloning Jul 2, 2017
@justinbmeyer
Copy link
Contributor Author

Ah, I think the problem is that only one steal-clone contextual module can be loaded:

if(!loader.has(name)) {

needs to be changed to:

if(!loader.has(localName)) {

@justinbmeyer
Copy link
Contributor Author

justinbmeyer commented Jul 2, 2017

Can confirm, this fixes it. Going to integrate the test and hopefully make a .pre release I can point CanJS to. Once @matthewp or @m-mujica give this a thumbs up, I'll make a real release.

@justinbmeyer justinbmeyer changed the title demonstrates error with cloning Fix bug with supporting multiple contextual modules of one type Jul 2, 2017
@matthewp
Copy link
Member

matthewp commented Jul 3, 2017

Great! Thanks.

@matthewp matthewp merged commit 5e35152 into master Jul 3, 2017
@matthewp matthewp deleted the 1221-dynamic-clones branch July 3, 2017 13:25
@matthewp
Copy link
Member

matthewp commented Jul 3, 2017

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