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

proposal: Deno.configPath(): string | undefined #19322

Closed
iuioiua opened this issue May 31, 2023 · 11 comments
Closed

proposal: Deno.configPath(): string | undefined #19322

iuioiua opened this issue May 31, 2023 · 11 comments

Comments

@iuioiua
Copy link
Contributor

iuioiua commented May 31, 2023

The purpose of this function is to expose the path of the Deno configuration file (deno.json) that the current script is using. Such functionality would be ideal in a script that needs to know where this file is.

For example, Fresh uses the deno.json file to start the server but uses a hardcoded path. Instead, it could call Deno.configPath(). This would become particularly useful when deno.json is in a different folder than other files that depend on it.

Related: #961

@crowlKats
Copy link
Member

Ref: #15482

@lino-levan
Copy link
Contributor

This would be nice. Perhaps this should be a just Deno.configPath (as a getter) since I don't think it can change during execution?

@timreichen
Copy link
Contributor

timreichen commented Jun 6, 2023

in the referenced issue by @crowlKats there is also the discussion of getting not just the config path but also the config data. That is not part of this proposal, but probably will be discussed in future proposal.
Therefore I wonder if it would make sense to add a config object Deno.config instead of separate getters or functions and instead of Deno.configPath, it should be something like Deno.config.path or Deno.config.filePath.

@iuioiua
Copy link
Contributor Author

iuioiua commented Jun 6, 2023

Having a Deno.configPath getter and Deno.Config interface would be sweet enough. It's easy enough to parse deno.json once its path is known. That'd be easier than implementing Deno.config within the runtime. Though, it wouldn't be too hard.

@timreichen
Copy link
Contributor

@iuioiua That would be true if it was just deno.json. But according to the doc the config file can also be .jsonc. So one would need to check for the file extension and then parse accordingly.
There are other advantages of providing the config data. Parsing alone doesn't resolve the data, which would be the point of exposing the config data. Two examples:

  • getting the resolved import map data, not the raw import map data or just a path pointing to the import map that then has to also be manually parsed.
  • getting the compilerOptions that includes the defaults set by deno.

And who knows if the config file changes again as it did by adding .jsonc support or inlining import maps. Providing the config data could potentially prevent third party mods from breaking.

@lino-levan
Copy link
Contributor

But according to the doc the config file can also be .jsonc. So one would need to check for the file extension and then parse accordingly.

You could use a .jsonc parser for both because it's just a superset of .json.

And who knows if the config file changes again as it did by adding .jsonc support or inlining import maps.

When configs were added in 1.14.0, they came out of the box with both .json and .jsonc. No breaking change there. I'd be very skeptical of adding a new file format (source: #11944).

Here's the way I see the config path / parsed config file argument:

Config Path

Pros Cons
More possibilities with the interface, since you know where the file is (you could modify it in a script) Opens up a bigger security hole than had existed in the past around config files and tasks
Much simpler implementation Opens up potential TOCTOA issues where the deno process loads, reads the config file, and then the file gets deleted

Config Data

Pros Cons
Simpler to use if you're just looking for what a specific setting is set to Implementation is a lot more complex
Doesn't reveal the location of the config file, making an attack harder Doesn't reveal the location of the config file, creating potential issues for frameworks who are interested in knowing that for legitimate reasons
Potentially allows to expose internal engine config :) Potentially allows to expose internal engine config :(

@timreichen
Copy link
Contributor

timreichen commented Jun 6, 2023

These are very nice pro/contra tables, I agree with most of the points made.

More possibilities with the interface, since you know where the file is (you could modify it in a script)

Maybe there is a use case where just the config file location is needed. If both data and path are exposed, there is nothing that stops the user from parsing it themselves if desired. But most use cases I can imagine also need to parse the file, so this seems like an unnecessary added step.

Implementation is a lot more complex

I guess so, but Deno resolves and uses the config file data internally, so couldn't it just be branched off and exposed from that?

Doesn't reveal the location of the config file, creating potential issues for frameworks who are interested in knowing that for legitimate reasons

Opens up potential TOCTOA issues where the deno process loads, reads the config file, and then the file gets deleted

These two issues would be solved when we declare that Deno.config contains the file path and config data read and used by deno on runtime start.

@iuioiua
Copy link
Contributor Author

iuioiua commented Jul 18, 2024

Closing in favor of #24628.

@iuioiua iuioiua closed this as not planned Won't fix, can't repro, duplicate, stale Jul 18, 2024
@dsherret
Copy link
Member

It's good we never added this API because with workspaces there are now many different configs.

@lino-levan
Copy link
Contributor

I think configPath would not have been a good api, but some way of resolving the deno config in a path would be something I would really like. It doesn't have to be a core Deno. api, but having it in std or something would be nice.

@dsherret
Copy link
Member

There probably needs to be a Wasm build of https://github.com/denoland/deno_config eventually (which is being renamed to deno_workspace soon).

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

No branches or pull requests

5 participants