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

import.meta.filename and import.meta.dirname #50

Closed
mcollina opened this issue Aug 2, 2023 · 33 comments
Closed

import.meta.filename and import.meta.dirname #50

mcollina opened this issue Aug 2, 2023 · 33 comments

Comments

@mcollina
Copy link

mcollina commented Aug 2, 2023

One of the most annoying things in the ESM implementation in Node.js is the lack of __filename and __dirname to work with paths from ESM files.

I would recommend we specify import.meta.dirname and import.meta.filename as optional shared metadata, returning the local paths in the filesystem as strings (and not file:// URLs).

For more context: nodejs/node#48740

@GeoffreyBooth
Copy link

GeoffreyBooth commented Aug 2, 2023

I think the desire for Node users at least would be for these two new methods to return the same strings that Node users are familiar with from CommonJS __filename and __dirname, values like:

  • __filename: /tmp/test/file.js
  • __dirname: /tmp/test

Note the lack of trailing slash on __dirname. That’s probably something we should standardize.

We also should standardize what import.meta.dirname and import.meta.filename are for modules that aren’t loaded from the filesystem, such as modules imported from an HTTPS URL. Should they be undefined? Empty strings? Getters that throw?

For reference:

@jasnell
Copy link
Contributor

jasnell commented Aug 2, 2023

My preference would be for runtimes (and wintercg) to...

  • Adopt the properties as implemented currently by bun. https://bun.sh/docs/api/import-meta
  • Define them such that they are optional and only relevant to runtimes that map modules to an underlying filesystem model (virtual or otherwise). Runtimes that do not map modules to an underlying filesystem model (or modules that are loaded from data: or https: paths, for instance) would simply omit these properties, relying instead on import.meta.url.

@GeoffreyBooth
Copy link

GeoffreyBooth commented Aug 3, 2023

Personally I think Bun can keep shipping import.meta.dir and import.meta.file like they do now, but the WinterCG spec defines import.meta.dirname and import.meta.filename to match Node’s __dirname and __filename. Bun can always support all four properties if they wish; fortunately they didn’t choose names that clash. I feel like there’s astronomically more code out there, and more developers, expecting the __filename and __dirname semantics.

@mcollina
Copy link
Author

mcollina commented Aug 3, 2023

The whole reason to add those for Node.js familiarity with __dirname and __filename.

@lucacasonato
Copy link
Member

Deno's stance on this is that we are generally opposed, because it results in different observable behaviour from within a module, depending on if it is served in HTTP (or other remote protocol), or the file system.

If the primary use-case is to reference assets relative to a given module, then a developer may use import.meta.file (which works locally), then publish to a CDN, and then the same code will not work anymore because remote modules have a https: specifier in the import.meta.url (and as such no import.meta.file).

Instead, developers should use import.meta.resolve, which returns a URL, and then do conditional logic based on the scheme of that URL. For example for file:// use readFile() (which is easy because readFile supports URLs), and for https:// use fetch().

Are there other use-cases that do not fall into the above class?

@pi0
Copy link

pi0 commented Aug 3, 2023

What if we conditionally expose import.meta.{filename,dirname} for modules that live in the filesystem? So for remote URLs, only depend on import.meta.url and avoid undesirable conditions?

Als a relevant issue since dealing with platform filesystem paths and url, will we keep Windows paths unnormalized with a backward slash for normalize to forward slashes? For context for unjs/mlly ESM utils, we make use of unjs/pathe which ensures normalized paths. It made life much easier to avoid edge-case conditions and in node

File URL requires platform normalization. The Node.js methods pathToFileURL, fileURLToPath are nor convenient to be used every place when depending on import.meta.url for fs paths. A normalized import.meta.dirname/import.meta.filename can make life much easier for developers.

@lucacasonato
Copy link
Member

My concern is primarily one of portability. If a developer relies on import.meta.filename, or import.meta.dirname and this works locally (because the module is on FS), and then they publish their module to a HTTP server and then it is imported as a https: module, then the code stops working. This breaks portability.

This is not just a concern for runtimes like Deno where local development happens on file URLs, and published modules are http URLs, but also for bundlers, and even node with custom loaders. In all of these cases, code may work in one scenario, and break in another.

This is a portability hazard -- we should not add more portability hazards, especially in a standards venue explicit created to promote portability.

@GeoffreyBooth
Copy link

Personally I think Bun can keep shipping import.meta.dir and import.meta.file like they do now, but the WinterCG spec defines import.meta.dirname and import.meta.filename to match Node’s __dirname and __filename.

I missed that Bun also has import.meta.path, which matches __filename. So if we want to just run with the names Bun has chosen, we could do import.meta.path for __filename and import.meta.dir for __dirname. Personally I feel like we should still ship import.meta.filename and import.meta.dirname because there are many more developers familiar with those names, but we can also ship both where import.meta.path and import.meta.filename are the same value, and import.meta.dir and import.meta.dirname are the same value.

@jasnell
Copy link
Contributor

jasnell commented Aug 3, 2023

What I would ask is that if Node.js and Deno want to ship import.meta.dir, import.meta.file, and import.meta.path, they collaborate on a spec document that defines the semantics clearly and independently of the specific runtime implementations. I would ask also that it deals with the portability footgun that @lucacasonato describes such that developers who are following the specs know what they may be getting themselves into should they use these properties.

Absent such a spec, these properties should be namespaced, e.g. import.meta.node.filename and import.meta.node.dirname, etc.

@GeoffreyBooth
Copy link

My concern is primarily one of portability. If a developer relies on import.meta.filename, or import.meta.dirname and this works locally (because the module is on FS), and then they publish their module to a HTTP server and then it is imported as a https: module, then the code stops working. This breaks portability.

Any code that uses fileURLToPath isn’t portable. If we don’t provide these helpers, it’ll just mean that people continue to use fileURLToPath and therefore they make code that’s not portable that way versus the import.meta.dirname way. I don’t think we should deny people convenience helpers just because they can’t be used everywhere; any code that intends to work with paths (whether via import.meta.dirname or via fileURLToPath) by definition isn’t intended to run from an HTTPS URL or in a browser environment.

Node also runs code from HTTPS URLs, and from data: URLs. So we have the same concern.

@GeoffreyBooth
Copy link

I think since Bun has already shipped this and Node intends to (either under import.meta.node or ideally at the top level, if we can find consensus) maybe the best way forward is to create an optional spec. Something like “for runtimes that want to ship import.meta properties to define the current module’s path and path-minus-filename (__filename and __dirname), the properties should be called ___ and ___ and have these specifications.” And it would define the details such as trailing slashes and how to handle non-filesystem modules. Then Bun and Node can each ship the feature according to the spec, and Deno can opt out; but if the Deno team changes their minds, they would add the feature according to the WinterCG spec. And I at least would welcome Deno’s involvement in drafting the spec, whether or not they plan to implement it.

@ljharb
Copy link
Member

ljharb commented Aug 3, 2023

Filesystem modules and url-fetched modules are fundamentally different; I’m not sure why that portability is expected.

@pi0
Copy link

pi0 commented Aug 4, 2023

@ljharb I agree that modules that would (optionally) depend on the filesystem (filename, dirname) are probably not designed to work from a CDN.

But I guess the standpoint of Deno is that with a stricter standard (imort.meta.{url,resolve}) modules can be always imported from a URL and also be able to operate on the local filesystem. Kinda enforcing developers to make CDN-friendly modules with cost of little bit harder DX. (ie: The new props can lead to encouraging developers to make libraries that are not CDN friendly)

I guess my question is that, at this point is "filesystem support" part of common-minimum-api spec? (because if not, i think as was talking in yesterday meeting we might simply delegate it to (scoped) runtime specific implementation rather than standard). There is so more to it. URL is (almost!) a web standard. Filesystem especially considering lovely windows, is not.

@ljharb
Copy link
Member

ljharb commented Aug 4, 2023

@pi0 developers can’t be forced to do things, and imo shouldn’t. That goal (importing from a URL) kind of works at a package granularity, but not at that of a file/module. CDN-friendliness is an antigoal to me.

@pi0
Copy link

pi0 commented Aug 4, 2023

Definitely agree with you @ljharb on that we cannot force developers nor runtimes not to do something and anyway runtimes such as Bun and Node.js would introduce their version of filename/dirname at some point. (I personally also will use dirname/filename meta if available but only for packages that are intended to be used locally not from a CDN like build tools or CLIs).

I think the earlier conclusions here were to either let runtimes (and developers) have their version and only encourage to use namespaced meta to make sure different implementations are clearly separated.

My counterpoint is, as a "minimum common standard" how much we can well define filename and dirname in a platform-agnostic way especially when many of current runtimes do not even have a definition of or support filesystem.

@Jarred-Sumner
Copy link

Bun is open to compromising here and making import.meta.dirname an alias of import.meta.dir.

filename is confusing because it is not the file's name. It is the absolute path to the file. filename only makes sense in the context of CommonJS' __filename, which is what the previous generation of developers know. It is less familiar to the current and next generation of developers who primarily author ES modules. import.meta.path makes more sense than import.meta.filename because it is the path to the file.

Bun is not in favor of namespaced import.meta properties like import.meta.bun or import.meta.node. I don't think that's a good experience for developers.

We don't have an opinion yet on what to do about modules which don't come from the filesystem. Instinctively, I'd think they would behave like a URL's pathname, excluding search params and the fragment. I think making them undefined or similar would unexpectedly break people's code.

@GeoffreyBooth
Copy link

Bun is open to compromising here and making import.meta.dirname an alias of import.meta.dir.

So I guess we at least agree on import.meta.dirname.

filename is confusing because it is not the file’s name. It is the absolute path to the file. filename only makes sense in the context of CommonJS’ __filename, which is what the previous generation of developers know. It is less familiar to the current and next generation of developers who primarily author ES modules. import.meta.path makes more sense than import.meta.filename because it is the path to the file.

I’m conflicted on this. On the one hand @Jarred-Sumner is absolutely correct, __filename was perhaps a poorly chosen name. On the other hand it’s been with us now for 13 years and is very widely known. Is it better to continue its legacy as import.meta.filename, building on the familiarity, or take this opportunity to correct the nomenclature and ship import.meta.path instead? (Or is there another alternative to consider? import.meta.path seems like a name potentially worth preserving as a namespace for future additions, like import.meta.path.basename and so on.) I can see the argument that if users need to type something new at all, because they need to add the import.meta prefix, then it doesn’t matter if they need to type a new name for the property; but on the other hand there are years’ worth of ecosystem READMEs mentioning __filename and it’s perhaps not obvious that the ESM equivalent is import.meta.path.

@jimmywarting
Copy link

jimmywarting commented Aug 16, 2023

I'm with @lucacasonato on this... developers should use import.meta.resolve instead
it's quicker approach to:

path.join(import.meta.dirname, 'dist')
// vs 
import.meta.resolve('./dist')

don't really see much use case for having dirname and filename

resolve works in browsers. import.meta.filename and import.meta.dirname dose not

@GeoffreyBooth
Copy link

developers should use import.meta.resolve instead

  • import.meta.resolve returns an URL string like file:///path/to/file.js.
  • import.meta.filename is a path string like /path/to/file.js.

They’re not interchangeable, hence the request.

@tniessen
Copy link
Member

I agree with @lucacasonato's points. The proposed properties are neither necessary nor portable and, in some ways, a step back. We probably shouldn't encourage their use even if they exist. However, enough folks seem to want these properties for convenience, despite these issues.

We don't have an opinion yet on what to do about modules which don't come from the filesystem. Instinctively, I'd think they would behave like a URL's pathname, excluding search params and the fragment. I think making them undefined or similar would unexpectedly break people's code.

On the other hand, explicitly making them undefined would at least force folks who use TypeScript to double-check if they really want to use properties that might not exist.

@ljharb
Copy link
Member

ljharb commented Aug 30, 2023

Not all code needs to be portable, and forcing that code to be less convenient seems a bit heavy handed.

@lucacasonato
Copy link
Member

My opinion on this has not changed since my last comment. The points I made above still hold true. Pushing for a import.meta property that is specific to modules loaded from disk is bad, because it discourages and prevents portability in cases where that is not warranted.

Also, I'd like to see how proposal authors intend to integrate this into TypeScript types. TypeScript has no mechanism to enable different import.meta based on how a file is loaded (and how could it, that is something not controlled by the module being imported, but by the importer!). If you take the heavy hammer approach and say "all modules are typed to have import.meta.path, then you are explicitly making the portability worse because developers can not even rely on the type system to tell them that import.meta.path is effectively optional and depends on how the module is loaded.

This seems entirely like we are trying to solve an education problem by introducing a footgun. This seems generally bad.

@benjamingr
Copy link
Member

Also, I'd like to see how proposal authors intend to integrate this into TypeScript types

I don't see why that's an issue? Presumably if someone loads @types/node that augments import.meta with module augmentation?

This seems entirely like we are trying to solve an education problem by introducing a footgun. This seems generally bad.

Users have been asking for this for a while and it's a real quality of life feature. There is only so long we can tell our users that the way they want to write their code (with paths in case paths are important) they're wrong and instead we need to enable them to author the software they want.

@ljharb
Copy link
Member

ljharb commented Sep 6, 2023

oh, i'd also add - i don't necessarily want my code to be portable to an environment i don't expect and support. If you want to use my code in a place I didn't intend, you should have to either fork it or ask me to allow for it, so I can add proper tests - otherwise there's no limit to what users will feel entitled to demand for free from open source maintainers.

@GeoffreyBooth
Copy link

I don’t see why that’s an issue? Presumably if someone loads @types/node that augments import.meta with module augmentation?

Node can run HTTPS and data:-URL modules, where these properties would be undefined, so it’s not as simple to say “if in Node environment, these exist.” And I assume TypeScript has no way of knowing whether a particular module as written will be run as a file: or as an https: module, so therefore there’s no way to declare that the properties definitively will or won’t exist. I would think they would just be typed as optional.

This would be a good thing, though: having these typed as potentially undefined would provide a hint to users about the portability risk, and push them to write conditional code that can handle either environment.

@bakkot
Copy link

bakkot commented Sep 6, 2023

Yeah for TS I'd expect filename?: string. If you are writing a node-only script you just do import.meta.filename!; if not you handle both cases.

@ljharb
Copy link
Member

ljharb commented Sep 6, 2023

Presumably the node types for import.meta could indicate that it's a string rather than absent.

@benjamingr
Copy link
Member

Node can run HTTPS and data:-URL modules, where these properties would be undefined, so it’s not as simple to say “if in Node environment, these exist.” And I assume TypeScript has no way of knowing whether a particular module as written will be run as a file: or as an https: module, so therefore there’s no way to declare that the properties definitively will or won’t exist. I would think they would just be typed as optional.

When going over "use HTTPS and data urls" use cases and "use import.meta.filename" use cases I suspect the intersection is tiny though?

@lucacasonato
Copy link
Member

I would love to hear from other folks who have a stake in portability and would be impacted by this, namely folks working on bundlers.

I concerned that this import.meta property will give users the expectation that bundlers can and do understand import.meta.path as a way to reference module relative assets, and include them in the bundler graph. They already have this expectation with import.meta.url. import.meta.filename would significantly increase the surface area of expressions that need to be statically analyzed.

cc @patak-dev @lukastaegert @devongovett @sokra (feel free to forward if anyone else is more interested in answering)

Presumably the node types for import.meta could indicate that it's a string rather than absent.

Except it is not always present in Node, as previously explained. So it can not be typed as "unconditional string" in Node either.

When going over "use HTTPS and data urls" use cases and "use import.meta.filename" use cases I suspect the intersection is tiny though?

This is totally besides the point? I don't care about the intersection use case - I would if I wanted import.meta.filename in the HTTPS and data urls use cases. I do not care about that though. I am concerned about the intersection of the set of "modules authored with file URLs that use import.meta.filename" and the set of "modules that could reasonably be run in https / data URL contexts if it weren't for import.meta.filename".

@guybedford
Copy link

Working on ncc, it was relatively straightforward to handle these kinds of expressions as "triggers" for asset analysis. I implemented an approach that walked each module AST, and when it found an expression deemed to be an 'asset source" - eg __dirname, __filename, import.meta.url etc, it would analyze that expression's parent statically, walking up the parent stack, as long as the static analyzer was able to analyze the current parent it would try the next. It would then note the best string value it got and use that URL / path as the path to be relocated. That parent node that was analyzed would then be replaced with the corresponding final bundle path to the asset. The relocation process would be reliable so long as the expression being replaced was analyzable and static. By restricting the static analysis to the right types of static analysis this seems to work, although it was certainly a mess of cases to handle all the various Node.js commonjs require scenarios at the time. This problem should in theory be more restricted if it strictly sticks to some formal subset of static analysis on something like the above, and also doesn't have to deal with existing compat to do that, but can create new relocation rules for the ecosystem going forward.

@devongovett
Copy link

I have no problem with supporting additional import.meta properties in Parcel. We already compile import.meta.url and CJS __filename/__dirname, so adding additional import.meta.dirname and import.meta.path would be easy for us.

For portability concerns, maybe import.meta.dirname and import.meta.path could just be based on the underlying import.meta.url? e.g. for http://example.com/foo/bar.js the dirname would be /foo and the path would be /foo/bar.js? But I'd also be fine with them being optional. Really depends what you want to do with these properties. Often it's things like reading a file relative to the current module, and obviously that won't work if the module is coming from http anyway.

@jimmywarting

This comment was marked as off-topic.

@jasnell
Copy link
Contributor

jasnell commented Apr 10, 2024

This have been handled by creating the import.meta registry.

@jasnell jasnell closed this as completed Apr 10, 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 a pull request may close this issue.