Skip to content
This repository has been archived by the owner on Sep 2, 2023. It is now read-only.

Feature: NodeJS contextual pathing use cases #121

Closed
guybedford opened this issue May 27, 2018 · 12 comments
Closed

Feature: NodeJS contextual pathing use cases #121

guybedford opened this issue May 27, 2018 · 12 comments
Labels

Comments

@guybedford
Copy link
Contributor

guybedford commented May 27, 2018

There are a number of workflows in NodeJS today that use __filename and __dirname to handle contextual paths relative to the current module itself.

Currently we are looking at exposing a import.meta.url which will be a file URL of the current module.

To use this contextual URL in NodeJS APIs requires a rather unwieldy conversion process along the lines of:

const filename = decodeURI(
  new URL(import.meta.url).pathname.substr(process.platform === 'win32' ? 1 : 0))
);

It would be great if we could allow a much more polished workflow here for users.

@benjamingr
Copy link
Member

What are the alternatives?

We could either accept import.meta.url to those APIs (which was mostly rejected from core APIs IIRC?) or expose another/more meta properties which would break compatibility with the web.

Another thing to consider is exposing a helper function for the conversion.

@guybedford
Copy link
Contributor Author

For reference re the rejections of getting file:// URL string support in Node core directly to support eg fs.readFile(import.meta.url) - nodejs/node#20944, nodejs/node#20950 were both blocked by security concerns.

@jdalton
Copy link
Member

jdalton commented May 27, 2018

@benjamingr I don't believe locking import.meta down to only APIs available in the browser is set in stone. In the past import.meta has been brought up as a possible place for other Node specific APIs. And talk was that as long as a path of interop between Node and browser (however narrow or not) existed then Node specific things would be OK.

@MylesBorins
Copy link
Member

Could we perhaps only allow URLs that include a particular symbol on the proto? We can then auto add that to import.meta.url... likely reduces the security concern since it is opt in

@guybedford
Copy link
Contributor Author

I think it would be good to try and explore carefully how __filename and __dirname are used in web-workflows so we can consider exactly what the universal use cases are here.

If anyone knows of good examples of libraries using __filename and __dirname on that web that would be a huge help.

As far as I recall, both __filename and __dirname are built into absolute URLs in the standard browserify workflow to support things like fetch(path.resolve(__dirname, 'resource')) but will have to double-check this again.

@benjamingr
Copy link
Member

@jdalton

I don't believe locking import.meta down to only APIs available in the browser is set in stone.

Oh definitely. I was under the impression import.meta.url aims to do one of those "non Node specific API"s that work across the browser and Node.

I'm personally fine with things like import.meta.require for interop.

Could we perhaps only allow URLs that include a particular symbol on the proto? We can then auto add that to import.meta.url... likely reduces the security concern since it is opt in

This is an interesting idea, we'd have to very carefully check that it doesn't violate user expectations though since new Worker(import.meta.url) would work but new Worker(import.meta.url + '') won't.

Is there any reason why import.meta.url can't be an actual URL by the way?

@guybedford
Copy link
Contributor Author

On the web, import.meta.url is definitely being defined as a string, so that really does mean we have to stick with that in Node if we want to align with the browser.

@benjamingr
Copy link
Member

On the web, import.meta.url is definitely being defined as a string,

I realize that this is the situation - but wouldn't making it a URL make our lives much easier?

I recall @iarna and @isaacs making a compelling argument file:// urls inside - would allowing passing a URL import.meta.url help at all with those concerns?

@bmeck
Copy link
Member

bmeck commented May 29, 2018

Do we have a comparison for the cases that are outside of what the fs compatibility for URLs supports?

Right now you can do some things like:

import fs from 'fs';
const files = fs.readdirSync(new URL('./', import.meta.url));
console.log(files);

Which don't actually require getting a raw path. I suspect most of the time this will affect child processes, but think that allowing child_process methods to take URLs (not url strings) might alleviate that as well. It would not solve the argv of child processes though, but we could also provide a generic fileURL -> string utility method somewhere that would solve any time people need to do this conversion manually.

@goto-bus-stop
Copy link

goto-bus-stop commented May 30, 2018

I think it would be good to try and explore carefully how __filename and __dirname are used in web-workflows so we can consider exactly what the universal use cases are here.
As far as I recall, both __filename and __dirname are built into absolute URLs in the standard browserify workflow to support things like fetch(path.resolve(__dirname, 'resource')) but will have to double-check this again.

the only way to really make __dirname and __filename useful with browserify is by adding a transform like brfs that can handle things like fs.readFile(path.join(__dirname, 'resource')) at compile time. browserify itself replaces __dirname and __filename with the absolute file paths on disk, so you can't use them to fetch() things. I'm not aware of any web libraries using those variables, except for ones that require brfs.

@SMotaal
Copy link

SMotaal commented Jun 12, 2018

Maybe this is a little late, but import.meta is an object, if import.meta does not throw, then import.meta.filename will simply be undefined in browsers, ie do what browser people do.

Is there a problem with including the filename and dirname in the InitializeImportMeta context for the implementer to decide if they want to throw them in?

At the same time, I think a very good performance metric to introduce would be the time spent in the URL.constructor vs. the number of calls to import.meta.url, we don't even need to worry about filtering constructor calls because statistically new URL in node is negligible imho.

@WebReflection
Copy link
Member

I wonder if as cross env solution this would work too:

const {
  pathname,
  filename = decodeURI(pathname.replace(/^\/([^:/]+:\/)/, '$1')),
  dirname = filename.replace(/\/[^/]*$/, '')
} = new URL(import.meta.url);

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

9 participants