Skip to content
This repository has been archived by the owner on Sep 25, 2019. It is now read-only.

Mimic how Node scans for modules © #376

Merged
merged 1 commit into from
Jun 30, 2017
Merged

Conversation

pirelenito
Copy link
Member

@pirelenito pirelenito commented Jun 29, 2017

  • Node modules takes precedence when colliding names;
  • Respects nested dependencies (This safely allows multiple versions of the same library to co-exist in a project).

More information: https://webpack.js.org/configuration/resolve/#resolve-modules

This will require a major version bump.

@pirelenito
Copy link
Member Author

@moriaam, as we discussed, this should fix the problem you are having.


describe('super test', function() {
it('should work', function() {
expect(MyProject.batata()).toEqual('sucesso')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this Portuguese as well, or just Italian?

import batata from 'batata'
import pastel from 'pastel'

export default {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It’s not clear to me how this example tests the problem at hand.

@pirelenito pirelenito force-pushed the mimic-node-module-scan branch 2 times, most recently from 59202b7 to 64a39a2 Compare June 29, 2017 11:49
@pirelenito
Copy link
Member Author

@xaviervia I just updated the tests so that they are more descriptive.

I also noticed that I didn't commit all the required files (because they were being git ignored).

@pirelenito pirelenito force-pushed the mimic-node-module-scan branch 2 times, most recently from 5c15c63 to a3726d4 Compare June 29, 2017 11:51
@pirelenito
Copy link
Member Author

Also, do you all agree this should be a major version? It could be considered a bug as well... I'm not 100% sure to be honest.

@pirelenito
Copy link
Member Author

@npejo, @Deschtex, I would love your input on this. Do you think this would be a change for a major version bump?

@pirelenito
Copy link
Member Author

@moriaam, you are not a contributor to Sagui, but I would also like to hear your thoughts. Do you think this is a breaking change?

@pirelenito
Copy link
Member Author

It could as well be considered a bug...

@moriaam
Copy link

moriaam commented Jun 29, 2017

I wanna say bugfix but it can mess up other projects so go safe with a major version.

@pirelenito
Copy link
Member Author

- Node modules takes precedence when colliding names;
- Respects nested dependencies (This safely allows multiple versions of the same library to co-exist in a project);
@pirelenito pirelenito changed the title Mimic how Node scans for modules, respecting nested dependencies © Mimic how Node scans for modules © Jun 29, 2017
@pirelenito
Copy link
Member Author

This is ready for the final review! Lets keep the same behavior as Node.js!

@pirelenito
Copy link
Member Author

Me and @npejo also tested this change in a big project and it worked fine. I'm merging and doing a major release.

@pirelenito pirelenito merged commit 11c90fe into v9 Jun 30, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants