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

Avoid the use of __dirname when importing files dynamically #216

Closed
paulogdm opened this issue Jan 14, 2019 · 9 comments
Closed

Avoid the use of __dirname when importing files dynamically #216

paulogdm opened this issue Jan 14, 2019 · 9 comments
Labels
enhancement New feature or request
Milestone

Comments

@paulogdm
Copy link
Member

Greetings!

Not sure if this issue would fit better on now-builders, but here we go.
Is there a way to avoid the necessity of __dirname when trying to import files locally?
This is our current usage:

 .
├── now.json
├── package.json
└── src
    ├── index.js
    └── temp.txt

index.js:

const {send} = require('micro')
const fs = require('fs')
const {promisify} = require('util')
const readFile = promisify(fs.readFile)
module.exports = async (req, res) => {
  const read = await readFile( __dirname + "/temp.txt")
  return send(res, 200, read.toString())
}

Many users face problems when doing readFile( "./temp.txt"), and the solution is above. But is there a way to avoid it?

@guybedford
Copy link
Contributor

A fix was attempted here in #217, but that had to be reverted unfortunately. Will hopefully come around on this again at some point soon.

@xcv58
Copy link

xcv58 commented Feb 13, 2019

IMO, It's not an enhancement but a bug fix!

No one can really onboard now 2.0 without this feature. Any complex server-side application needs files not only ts,js, json, graphql files are very common to use in backend.

@revskill10
Copy link

revskill10 commented Feb 13, 2019

+1 .
The lack of this feature blocks incremental migration from a legacy node-server to Now 2 platform.
In many case, we want to:

  • Read file synchronously from local system.
  • Use path.resolve when building (both build-time and run-time), for example, in a (req, res) function when we want to get the javascript chunks extracted at build-time.

knownasilya added a commit to loaves-and-fishes/type-graphql-test that referenced this issue Feb 14, 2019
@guybedford
Copy link
Contributor

@revskill10 can you share the exact code patterns you are using here? That would help a lot as this issue is all about finding the specific triggers that won't "overclassify".

Is the pattern fs.readFile(path.resolve('x'))? If so we can certainly add a path.resolve trigger.

@revskill10
Copy link

@guybedford I'm trying to deploy this example from loadable/components here
Note the part of nodeStats trying to resolve the loadable-stats.json

const nodeStats = path.resolve(
  __dirname,
  '../../public/dist/node/loadable-stats.json',
)

How Now 2 offer this ability ?

@guybedford
Copy link
Contributor

@revskill10 that code example should definitely be working fine because of the usage of __dirname. If it isn't please post an exact replication as we do have tests for this case that are working fine.

@iaincollins
Copy link

iaincollins commented Feb 26, 2019

I ran into this specific problem when using the npm module unfluff.

@guybedford I think fs.readFile(path.resolve('x')) would be a welcome edition, but does not resolve the issue, as even with that some modules will still not work.

A repoducable example that fails - when unfluff() is called an exception is thrown because it cannot load one of the text files in it's 'data' directory:

const { send } = require('micro')
const microQuery = require('micro-query')
const unfluff = require('unfluff')
const fetch = require('node-fetch')

module.exports = async (req, res) => {
  const query = microQuery(req)

  if (!query.url)
    return send(res, 400, { error: 'URL parameter missing' })
  
  const fetchRes = await fetch(query.url)
  const text = await fetchRes.text()
  
  return send(res, 200, {
      url: query.url,
      ...unfluff(text)
  })
}

Exception:

{
  "errno": -2,
  "code": "ENOENT",
  "syscall": "open",
  "path": "/var/task/user/api/content/data/stopwords/stopwords-en.txt"
}

(Note: Obviously this a bad example of a thing to expose publicly, but is simplified from what I am trying to do for illustration.)

I tried and failed to find a way to patch the module for this. The module tries to load the txt files at run time using a variable name for the language code.

Even adding a dozen or so readFile() commands with the path to them to the main entry point of the module (which I forked off) didn't resolve the issue - they were just not getting bundled.

I even tried including all the files needed alongside my function like this, and referencing them from my own function in the hope the bundler would see them (and so they would be accessible when the module tried to load them) but that didn't work either and resulted in the same error:

fs.readFile(__dirname + '/data/stopwords/stopwords-ar.txt');
fs.readFile(__dirname + '/data/stopwords/stopwords-bg.txt');
fs.readFile(__dirname + '/data/stopwords/stopwords-cs.txt');
fs.readFile(__dirname + '/data/stopwords/stopwords-da.txt');
fs.readFile(__dirname + '/data/stopwords/stopwords-de.txt');
fs.readFile(__dirname + '/data/stopwords/stopwords-en.txt');
fs.readFile(__dirname + '/data/stopwords/stopwords-es.txt');
fs.readFile(__dirname + '/data/stopwords/stopwords-fi.txt');
fs.readFile(__dirname + '/data/stopwords/stopwords-fr.txt');
fs.readFile(__dirname + '/data/stopwords/stopwords-hu.txt');
fs.readFile(__dirname + '/data/stopwords/stopwords-id.txt');
fs.readFile(__dirname + '/data/stopwords/stopwords-it.txt');
fs.readFile(__dirname + '/data/stopwords/stopwords-ko.txt');
fs.readFile(__dirname + '/data/stopwords/stopwords-nb.txt');
fs.readFile(__dirname + '/data/stopwords/stopwords-nl.txt');
fs.readFile(__dirname + '/data/stopwords/stopwords-no.txt');
fs.readFile(__dirname + '/data/stopwords/stopwords-pl.txt');
fs.readFile(__dirname + '/data/stopwords/stopwords-pt.txt');
fs.readFile(__dirname + '/data/stopwords/stopwords-ru.txt');
fs.readFile(__dirname + '/data/stopwords/stopwords-sv.txt');
fs.readFile(__dirname + '/data/stopwords/stopwords-th.txt');
fs.readFile(__dirname + '/data/stopwords/stopwords-tr.txt');
fs.readFile(__dirname + '/data/stopwords/stopwords-zh.txt');

I don't see any way to work around this and I would be grateful even for hacky workaround.

The module in this case is sufficiently complicated to be non-trivial to try and maintain a fork with different behaviour. I'll port it to be compatible if I have to, but I'm loathe to as I won't be able to maintain it properly.

@guybedford
Copy link
Contributor

@iaincollins it seems the code you're referring to here is the following pattern:

getFilePath = function (language) {
  return path.join(__dirname, '..', 'data', 'stopwords', 'stopwords-' + language + '.txt');
};

note that this is actually __dirname-based, so rather the problem is one of __dirname variations. I've created #297 separately to track.

@styfle
Copy link
Member

styfle commented May 17, 2019

Fixed in #378

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants