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

replace __dirname in favor of process.cwd in diff command web flag #2856

Merged
merged 4 commits into from
Aug 26, 2024

Conversation

prochafilho
Copy link
Contributor

@prochafilho prochafilho commented Aug 26, 2024

🍗 Description

What does this PR do? Anything folks should know?

A bit of context: to facilitate the consumption of @useoptic/optic, we package it using pkg to make it an executable.

If I run the diff command using native TypeScript compilers such as ts-node, I have no issue because the final HTML file resolves to a local file in my filesystem, for example, file:///Users/xxxxxxx/work/tools/tool-api-standards/node_modules/@useoptic/optic/web/build/index.html#W0cZF0U

But because we package using pkg, it prefixes the local file path with snapshot, so it becomes:
file:///snapshot/tool-api-standards/node_modules/@useoptic/optic/web/build/index.html#W0cZF0U

During packaging process pkg collects project files and places them into executable. It is called a snapshot. At run time the packaged application has access to snapshot filesystem where all that files reside.
Packaged files have /snapshot/ prefix in their paths (or C:\snapshot\ in Windows). If you used pkg /path/app.js command line, then __filename value will be likely /snapshot/path/app.js at run time. __dirname will be /snapshot/path as well.

I think it is caused by maybeChangelogUrl found in the following line in combination with our use of pkg to make executables: maybeChangelogUrl

pkg recommends to use process.cwd() instead of __dirname

On the other hand, to access a real file system at run time (pick up a user’s external javascript plugin, JSON configuration or even get a list of the user’s directory) you should take process.cwd() or path.dirname(process.execPath)

One can assert the following is true: (process.execPath) === __dirname === true

📚 References

Links to relevant docs (Notion, Twist, GH issues, etc.), if applicable.
https://www.npmjs.com/package/pkg#snapshot-filesystem
https://www.npmjs.com/package/ts-node

👹 QA

How can other humans verify that this PR is correct?

When running diff with the --web flag, it should open a browser window with the results.

@prochafilho prochafilho requested a review from acunniffe as a code owner August 26, 2024 14:11
@acunniffe acunniffe enabled auto-merge August 26, 2024 14:57
@acunniffe acunniffe added this pull request to the merge queue Aug 26, 2024
Merged via the queue into opticdev:main with commit 03f4a9a Aug 26, 2024
3 checks passed
acunniffe pushed a commit that referenced this pull request Aug 26, 2024
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.

3 participants