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

Tests for problems introduced in ~resolve@1.1.5 and fixed in resolve@1.1.6 #1139

Merged
merged 1 commit into from Feb 8, 2019

Conversation

Projects
None yet
2 participants
@dominykas
Copy link
Contributor

dominykas commented Feb 24, 2015

When a module is exposed, it should still resolve the way it would normally do, i.e. with/without extension and directories should fall back to index, and index from a directory should be accepted with/without extension too.

In this PR there are the tests that will fail - I am not able to run them reliably for some reason, so with resolve@1.1.5 they simply hang, however they will pass if you npm install resolve@1.1.2

I'm not sure how to make them pass - that, I believe, belongs in the resolve module - I'll create a relevant issue there, mostly because I've no idea how things are wired up.

The following tests are added, where x is a file and y is a folder with an index.js inside:

  • b.require('./x', {expose: 'xyz'});
  • b.require('./y', {expose: 'xyz'});
  • b.require('./y/index', {expose: 'xyz'});
  • b.require('./y/index.js', {expose: 'xyz'});

@dominykas dominykas changed the title should resolve modules when exposing folders Regression: should resolve modules when exposing folders Feb 24, 2015

@dominykas dominykas changed the title Regression: should resolve modules when exposing folders Regression: should resolve modules when exposing folders (failing tests attached) Feb 24, 2015

@dominykas dominykas force-pushed the insidewarehouse:resolve-exposed-folders branch from 975a3a2 to 7a16a7d Apr 21, 2015

@dominykas dominykas changed the title Regression: should resolve modules when exposing folders (failing tests attached) Tests for problems introduced in ~resolve@1.1.5 and fixed in resolve@1.1.6 Apr 21, 2015

@dominykas

This comment has been minimized.

Copy link
Contributor Author

dominykas commented Apr 21, 2015

After rebasing tests now pass - I assume the tests are still worth merging into the suite?

@dominykas

This comment has been minimized.

Copy link
Contributor Author

dominykas commented Apr 13, 2018

I suppose this is not going in.

@dominykas dominykas closed this Apr 13, 2018

@dominykas dominykas deleted the insidewarehouse:resolve-exposed-folders branch Apr 13, 2018

@dominykas dominykas restored the insidewarehouse:resolve-exposed-folders branch Apr 13, 2018

@ljharb

This comment has been minimized.

Copy link
Contributor

ljharb commented Apr 13, 2018

It’d be great if this went in.

@ljharb ljharb reopened this Apr 13, 2018

@ljharb

This comment has been minimized.

Copy link
Contributor

ljharb commented Apr 13, 2018

@dominykas i know this is old, but if you could rebase it, I’d love to see it go in. It will help ensure that resolve doesn’t break behavior in browserify.

@dominykas

This comment has been minimized.

Copy link
Contributor Author

dominykas commented Apr 13, 2018

OK, I'll see what I can do.

@dominykas dominykas closed this Feb 7, 2019

@dominykas dominykas deleted the insidewarehouse:resolve-exposed-folders branch Feb 7, 2019

@dominykas dominykas restored the insidewarehouse:resolve-exposed-folders branch Feb 7, 2019

@dominykas dominykas reopened this Feb 7, 2019

when a module is exposed, it should still resolve the way it would no…
…rmally do, i.e. with/without extension and directories should fall back to index, and index from a directory should be accepted with/without extension too

@dominykas dominykas force-pushed the insidewarehouse:resolve-exposed-folders branch from f6d6728 to f13b713 Feb 7, 2019

@dominykas

This comment has been minimized.

Copy link
Contributor Author

dominykas commented Feb 7, 2019

I've no recollection of details of what happened there, but let's see if rebasing makes the tests pass :)

@ljharb

ljharb approved these changes Feb 8, 2019

Copy link
Contributor

ljharb left a comment

More tests, tests that pass, I'm not seeing a downside

@ljharb ljharb merged commit 7ad39ce into browserify:master Feb 8, 2019

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@dominykas dominykas deleted the insidewarehouse:resolve-exposed-folders branch Feb 8, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment