install package with file: protocol as default if it exists in the filesystem and using fs.exists instead of using fs.existsSync #2723

voxsim · 2017-02-19T11:49:16Z

This pull request should resolve #605 and it is related to #2684.

In this case we use fs.exists instead of using fs.existsSync.

cheers,
voxsim

bestander

a few nits

bestander · 2017-02-19T23:22:37Z

src/package-request.js

+
+  async normalize(pattern: string): any {
+    const {name, range, hasVersion} = PackageRequest.normalizePattern(pattern);
+    const new_range = await this.normalizeRange(range);


nit: do we use underscore for word separation anywhere in Yarn?
I think we should stick to the same camelCase.

done, resolved ;)

bestander · 2017-02-19T23:25:10Z

src/package-request.js

+      return Promise.resolve(pattern);
+    }
+
+    if (await fs.exists(path.join(this.config.cwd, pattern))) {


How often is this IO operation is used?
Can we infer if pattern looks like a file first before touching the disk if this happens often?

After some over-thinking we can do something like this:

if (pattern.includes(':') || pattern.includes('@') || PackageRequest.getExoticResolver(pattern)) { return Promise.resolve(pattern); }

this should give us enough confidence that the pattern may be a file if we go ahead in the method. Do you like it? Do you think is enough?

…lesystem

bestander · 2017-02-20T16:00:48Z

Thanks for the fix, @voxsim and for doing a quick turnaround!

kjbkjb · 2017-04-14T19:54:03Z

greetings.

Very new to yarn. Loving it.

I believe this particular PR has broken local tgz file installation.

  "dependencies": {
    "bell": "^8.6.0",
    "boom": "^4.3.1",
    "code": "^4.0.0",
    "confidence": "^3.0.2",
    "config": "^1.12.0",
    "crumb": "^6.0.3",
    "dustjs-linkedin": "^2.7.2",
    "feature-toggles": "file:.modules/feature-toggles-3.0.3.tgz",
    "global-session": "file:.modules/global-session-3.0.0.tgz",

This mechanism worked in 0.22, but fails in 0.23

Now we are seeing:

yarn
yarn install v0.23.2
[1/4] 🔍  Resolving packages...
error An unexpected error occurred: "https://npm-proxy.fury.io/.../.modules/sportngin-feature-toggles-3.0.3.tgz: Request failed \"404 Not Found\"".
info If you think this is a bug, please open a bug report with the information provided in "/yarn-error.log".
info Visit https://yarnpkg.com/en/docs/cli/install for documentation about this command.

Happy to file a bug or take any advise

voxsim · 2017-04-14T19:59:59Z

hey @kjbkjb, have you tried with another version of the same package? Because the issue it does not seem related, i mean really the server doesn't have that version of the package. Btw if you think it is a real issue, maybe it's better to open an issue and link this pull request ;)

kjbkjb · 2017-04-14T20:45:19Z

I've tried building several fresh versions. if the version did not exist before and it was not listed in the lock file, version 0.22 would find the file in the local file system, but 0.23 tries to look up the module on npm and ignores the local file. This pr was dealing with file installation so I thought it looked suspicious. I'll file an issue.

voxsim changed the title ~~install package with file: protocol as default if it exists in the fi…~~ install package with file: protocol as default if it exists in the filesystem and using fs.exists instead of using fs.existsSync Feb 19, 2017

voxsim mentioned this pull request Feb 19, 2017

install package with file: protocol as default if it exists in the filesystem #2684

Closed

bestander requested changes Feb 19, 2017

View reviewed changes

voxsim force-pushed the install_with_file_protocol_as_default_with_fs_async branch from b7cc7d5 to 80fe1c0 Compare February 20, 2017 14:09

install package with file: protocol as default if it exists in the fi…

ce63d24

…lesystem

voxsim force-pushed the install_with_file_protocol_as_default_with_fs_async branch from 80fe1c0 to ce63d24 Compare February 20, 2017 14:11

bestander merged commit c001bb2 into yarnpkg:master Feb 20, 2017

kjbkjb mentioned this pull request Apr 17, 2017

Installation of Local Filesystem tgz Modules Fails in 0.23 #3168

Closed

mikemaccana mentioned this pull request Sep 11, 2017

yarn attempts to reinstall already-installed private packages #4399

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

install package with file: protocol as default if it exists in the filesystem and using fs.exists instead of using fs.existsSync #2723

install package with file: protocol as default if it exists in the filesystem and using fs.exists instead of using fs.existsSync #2723

voxsim commented Feb 19, 2017

bestander left a comment

bestander Feb 19, 2017

voxsim Feb 20, 2017

bestander Feb 19, 2017

voxsim Feb 20, 2017

bestander commented Feb 20, 2017

kjbkjb commented Apr 14, 2017 •

edited

Loading

voxsim commented Apr 14, 2017 •

edited

Loading

kjbkjb commented Apr 14, 2017

install package with file: protocol as default if it exists in the filesystem and using fs.exists instead of using fs.existsSync #2723

install package with file: protocol as default if it exists in the filesystem and using fs.exists instead of using fs.existsSync #2723

Conversation

voxsim commented Feb 19, 2017

bestander left a comment

Choose a reason for hiding this comment

bestander Feb 19, 2017

Choose a reason for hiding this comment

voxsim Feb 20, 2017

Choose a reason for hiding this comment

bestander Feb 19, 2017

Choose a reason for hiding this comment

voxsim Feb 20, 2017

Choose a reason for hiding this comment

bestander commented Feb 20, 2017

kjbkjb commented Apr 14, 2017 • edited Loading

voxsim commented Apr 14, 2017 • edited Loading

kjbkjb commented Apr 14, 2017

kjbkjb commented Apr 14, 2017 •

edited

Loading

voxsim commented Apr 14, 2017 •

edited

Loading