Skip to content

fix(dts): adapt ReplaceInFileConfig interface to actual implementation #107

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Apr 25, 2020
Merged

fix(dts): adapt ReplaceInFileConfig interface to actual implementation #107

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
3c5d857
fix(dts): adapt ReplaceInFileConfig interface to actual implementation
antongolub Feb 6, 2020
22364d3
chore(dts): redeclare sync method outside ns to handle circular defin…
antongolub Feb 6, 2020
531b1ec
feat: use named export for replaceInFile
antongolub Feb 19, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion lib/replace-in-file.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ function replaceInFile(config, cb) {
/**
* Sync API
*/
replaceInFile.sync = function(config) {
function replaceInFileSync(config) {

//Parse config
config = parseConfig(config);
Expand All @@ -84,5 +84,9 @@ replaceInFile.sync = function(config) {
return paths.map(path => replaceSync(path, from, to, config));
};

replaceInFile.replaceInFile = replaceInFile; // Self-reference to support named import
replaceInFile.replaceInFileSync = replaceInFileSync;
replaceInFile.sync = replaceInFileSync;
Copy link
Contributor

@JonnySpruce JonnySpruce Apr 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the benefit of having both sync and replaceInFileSync?

Copy link
Contributor Author

@antongolub antongolub Apr 24, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sync is just a legacy alias for backward compatibility
New named exports let us to avoid using of default module reference:

// Before
import replaceInFile, {sync as replaceInFileSync} from 'replace-in-file'
// After
import {replaceInFile, replaceInFileSync} from 'replace-in-file'


//Export
module.exports = replaceInFile;
20 changes: 19 additions & 1 deletion lib/replace-in-file.spec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
/**
* Dependencies
*/
const replace = require('./replace-in-file');
import replace, {sync, replaceInFile, replaceInFileSync} from './replace-in-file'
Copy link
Owner

@adamreisnz adamreisnz Apr 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am a bit surprised that this works and passes the travis build in Node versions 8 and 10?
I thought ES6 style import were only available in Node from version 12 onwards.

const fs = require('fs');
const writeFile = Promise.promisify(fs.writeFile);
const deleteFile = Promise.promisify(fs.unlink);
Expand Down Expand Up @@ -1151,4 +1151,22 @@ describe('Replace in file', () => {
expect(results[0].numReplacements).to.equal(0);
});
});

describe('module export', () => {
it('default module export refers to async replace implementation', () => {
expect(replace).to.be.a('function');
});

it('exports named replaceInFile, replaceInFileSync and sync from module facade', () => {
expect(replaceInFile).to.be.a('function');
expect(replaceInFileSync).to.be.a('function');
expect(sync).to.be.a('function');
});

it('exposes inner functions as own fields of replace', () => {
expect(replace.replaceInFile).to.equal(replace);
expect(replace.sync).to.equal(replaceInFileSync);
expect(replace.replaceInFileSync).to.equal(replaceInFileSync);
});
})
});
17 changes: 13 additions & 4 deletions types/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,27 @@

declare module 'replace-in-file' {
function replaceInFile(config: ReplaceInFileConfig): Promise<ReplaceResult[]>;
function replaceInFile(config: ReplaceInFileConfig, cb: (error: Error, results: ReplaceResult[]) => void): void;
export function replaceInFile(config: ReplaceInFileConfig): Promise<ReplaceResult[]>;
export function replaceInFile(config: ReplaceInFileConfig, cb: (error: Error, results: ReplaceResult[]) => void): void;
export default replaceInFile;

namespace replaceInFile {
export function sync(config: ReplaceInFileConfig): ReplaceResult[];
export function replaceInFileSync(config: ReplaceInFileConfig): ReplaceResult[];
export function replaceInFile(config: ReplaceInFileConfig): Promise<ReplaceResult[]>;
export function replaceInFile(config: ReplaceInFileConfig, cb: (error: Error, results: ReplaceResult[]) => void): void;
}

export function sync(config: ReplaceInFileConfig): ReplaceResult[];
export function replaceInFileSync(config: ReplaceInFileConfig): ReplaceResult[];

export type From = string | RegExp | FromCallback;
export type To = string | ToCallback;

export interface ReplaceInFileConfig {
files: string | string[];
ignore?: string | string[];
from: string | RegExp | string[] | RegExp[] | FromCallback;
to: string | string[] | ToCallback;
from: From | Array<From>;
to: To | Array<To>;
Copy link
Contributor

@JonnySpruce JonnySpruce Apr 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would this change not also suggest an Array of FromCallback type could happen?

Copy link
Contributor Author

@antongolub antongolub Apr 24, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's possible, but let's inspect the current implementation:

  1. replaceInFileSync invokes parseConfig
  2. parseConfig optionally handles files and ignore fields, injects defaultConfig and returns it back
Object.assign({}, defaults, config)
  1. Then config params are passed to replaceSync
const {files, from, to, dry, verbose} = config;
...
paths.map(path => replaceSync(path, from, to, config));
  1. and then to makeReplacements
makeReplacements(
    contents, from, to, file, countMatches
  );
  1. where from turns into array
  if (!Array.isArray(from)) {
    from = [from];
  }
  1. which is reduced
const newContents = from.reduce((contents, item, i) => {
  // ...
    if (typeof item === 'function') {
      item = item(file);
    }
  // ...
}

It looks that Array<FromCallback> is supported.

countMatches?: boolean;
allowEmptyPaths?: boolean,
disableGlobs?: boolean,
Expand Down