broccolijs / fs-merger Goto Github PK

View Code? Open in Web Editor NEW

2.0 2.0 6.0 164 KB

Library which performs broccoli merging and funneling without actually copying files.

JavaScript 61.41% TypeScript 38.59%

fs-merger's People

Contributors

Stargazers

Watchers

Forkers

stefanpenner thoov chriseppstein swornimbarahi kturney isabella232

fs-merger's Issues

Avoid calling getRootAndPrefix repeated with the same input

It seems that every fs operation will be calling getRootAndPrefix:

https://github.com/SparshithNR/fs-merger/blob/021b9075fe08d28edb99c550bdae6b41080e3fa4/index.js#L59

We should avoid calling this so many times.

readFileSync returns undefined when the file not found.

It should raise an error.

`readFileMeta` seems unused?

https://github.com/SparshithNR/fs-merger/blob/021b9075fe08d28edb99c550bdae6b41080e3fa4/index.js#L99-L134

The broccoli PR does not use it AFAICT

Move to BroccoliJS org

@SparshithNR - Since this is used by default by Broccoli, I think we should probably move it into the BroccoliJS org. Any objections?

Accessing `at` using `FSMerger.fs` proxy causes error.

let fsMerger = new FSMerger([`a`,`b`]);
let fileAt = fsMerger.fs.at(1).readFileSync('test.txt'); //errors

TypeError [ERR_INVALID_ARG_TYPE]: The "path" argument must be of type string. Received type number
      at validateString (internal/validators.js:112:11)
      at Object.isAbsolute (path.js:1030:5)
      at FSMerger.handleOperation (node_modules/fs-merger/dist/index.js:51:15)

Avoid private API's in broccoli nodes

https://github.com/SparshithNR/fs-merger/blob/021b9075fe08d28edb99c550bdae6b41080e3fa4/index.js#L20-L23

The `dist` folder is missing in `3.2.0`

😅
@thoov, pinging you before people start freaking out

`FSMerger.prototype.entries()` should likely not error

Running: this.input.entries() currently throws with:

> this.input.entries()
NodeError: The "path" argument must be of type string. Received type undefined
    at validateString (internal/validators.js:125:11)
    at Object.isAbsolute (path.js:1136:5)
    at FSMerger.handleOperation (/Users/spenner/src/stefanpenner/gql-experiment/node_modules/fs-merger/dist/index.js:54:39)
    at eval (eval at build (/Users/spenner/src/stefanpenner/gql-experiment/index.js:7:5), <anonymous>:1:12)
    at GQL.build (/Users/spenner/src/stefanpenner/gql-experiment/index.js:7:5)
    at TransformNodeWrapper.build (/Users/spenner/src/stefanpenner/gql-experiment/node_modules/broccoli/dist/wrappers/transform-node.js:70:39)
    at pipeline.then (/Users/spenner/src/stefanpenner/gql-experiment/node_modules/broccoli/dist/builder.js:118:30)
    at process._tickCallback (internal/process/next_tick.js:68:7)

This isn't super helpful, it should likely rather just assume the caller wants all relevant entries.

this.input.entries() should likely perform like this.input.entries('.')

sh

Error about unsupported API's seems incorrect

https://github.com/SparshithNR/fs-merger/blob/021b9075fe08d28edb99c550bdae6b41080e3fa4/index.js#L72

This error that is thrown suggests that only whitelisted operations are allowed. The operations in this constant are:

https://github.com/SparshithNR/fs-merger/blob/021b9075fe08d28edb99c550bdae6b41080e3fa4/index.js#L6-L12

However, we also allow the end consumer to call any method on the fs-merger itself:

https://github.com/SparshithNR/fs-merger/blob/021b9075fe08d28edb99c550bdae6b41080e3fa4/index.js#L48-L55

This means that users in broccoli-land could do this.input.readdir (note: not the sync version) and get this implementation:

https://github.com/SparshithNR/fs-merger/blob/021b9075fe08d28edb99c550bdae6b41080e3fa4/index.js#L156

Accessing at using fs proxy yeilds error.

When multiple trees contain a given file, which should win?

In general with broccoli-merge-trees rules, the "last one wins" (but only if you've specified { overwrite: true }. This library also supports "last wins", but it still reads from each tree. See implementation here:

https://github.com/SparshithNR/fs-merger/blob/021b9075fe08d28edb99c550bdae6b41080e3fa4/index.js#L78-L89

If we do indeed intend to support "last wins" we should update the implementations to avoid reading files only to throw away those results (e.g. we should iterate backwards and return the first match).