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

chore: normalize file path #910

Merged
merged 2 commits into from
Nov 16, 2023
Merged

Conversation

ylmin
Copy link
Contributor

@ylmin ylmin commented Oct 31, 2023

Description of the Changes

  • normalize File Path

PR Preview URL

No

Check List

  • Changes have been done against dev branch, and PR does not conflict
  • PR title follows the convention: <docs/feat/fix/chore>(optional scope): <description>, e.g: fix: minor typos in code

This change is Reviewable

@stoobie
Copy link
Collaborator

stoobie commented Nov 12, 2023

Description of the Changes

  • normalize File Path

@ylmin Can you please provide more information? What is the problem that you are trying to fix? How does this change fix the problem?

@ylmin ylmin closed this Nov 12, 2023
@ylmin ylmin reopened this Nov 12, 2023
@ylmin
Copy link
Contributor Author

ylmin commented Nov 12, 2023

Description of the Changes

  • normalize File Path

@ylmin Can you please provide more information? What is the problem that you are trying to fix? How does this change fix the problem?

Absolute paths vs Relative paths: path.resolve(__dirname, './file.js') returns the absolute path of 'file.js', while './file.js' returns a relative path relative to the current working directory.

Ensuring Reliable File Positioning: path.resolve(__dirname, './file.js') ensures that the position of the file is relative to the current module file, which is not affected by the startup path of the Node.js application. On the other hand, directly using './file.js' as a relative path might be affected by the startup path.

Therefore, using path.resolve(__dirname, './file.js') can more reliably obtain the absolute path of a file.

@ylmin
Copy link
Contributor Author

ylmin commented Nov 12, 2023

Description of the Changes

  • normalize File Path

@ylmin Can you please provide more information? What is the problem that you are trying to fix? How does this change fix the problem?

Understood! Here's the difference between using path.resolve with and without __dirname:

When using path.resolve without __dirname, the function will resolve the path based on the current working directory (CWD) of the Node.js process. In other words, if you provide a relative path without __dirname, the function will assume that the path is relative to the CWD.

On the other hand, when using path.resolve with __dirname, the function will resolve the path based on the directory where the current module file is located. In other words, if you provide a relative path with __dirname, the function will assume that the path is relative to the directory where the module file is located.

The difference lies in how the function interprets the relative path. Without __dirname, the function assumes that the path is relative to the CWD, which may not be the desired behavior in some cases. For example, if you have a folder structure like this:

my_project/
│ index.js
│ node_modules/
│ lib/
└── main.js
If you call path.resolve(__dirname, '../lib/my_file.js'), without using __dirname, the function will resolve the path to /./lib/my_file.js, which may not be what you intended. However, if you use path.resolve(__dirname, 'my_file.js'), the function will resolve the path to my_file.js in the lib folder, which is the desired behavior.

In summary, using path.resolve with __dirname can help you write more predictable and robust code by ensuring that the relative path is resolved based on the correct directory.

@stoobie
Copy link
Collaborator

stoobie commented Nov 15, 2023

@ylmin Can you suggest how I can test this to make sure it doesn't break anything? Sorry, I'm not much of a JS dev.

@stoobie stoobie changed the base branch from dev to ylmin/normalize_file_path November 16, 2023 08:09
@stoobie stoobie merged commit c424331 into starknet-io:ylmin/normalize_file_path Nov 16, 2023
1 of 2 checks passed
stoobie added a commit that referenced this pull request Nov 28, 2023
* refactor: normalize File Path (#910)

Merging this to a new PR to force generating a preview.

Co-authored-by: Steve Goodman <[email protected]>

* Update js/algolia-index.js

* Update js/algolia-index.js

---------

Co-authored-by: Jack <[email protected]>
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