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

feat: add update method as safer alternative to overwrite #212

Merged
merged 7 commits into from Oct 5, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
20 changes: 15 additions & 5 deletions README.md
Expand Up @@ -43,10 +43,10 @@ import fs from 'fs'

const s = new MagicString('problems = 99');

s.overwrite(0, 8, 'answer');
s.update(0, 8, 'answer');
s.toString(); // 'answer = 99'

s.overwrite(11, 13, '42'); // character indices always refer to the original string
s.update(11, 13, '42'); // character indices always refer to the original string
s.toString(); // 'answer = 42'

s.prepend('var ').append(';'); // most methods are chainable
Expand Down Expand Up @@ -135,6 +135,10 @@ The `options` argument can have an `exclude` property, which is an array of `[st

**DEPRECATED** since 0.17 – use `s.prependRight(...)` instead

### s.isEmpty()

Returns true if the resulting source is empty (disregarding white space).

### s.locate( index )

**DEPRECATED** since 0.10 – see [#30](https://github.com/Rich-Harris/magic-string/pull/30)
Expand All @@ -149,10 +153,12 @@ Moves the characters from `start` and `end` to `index`. Returns `this`.

### s.overwrite( start, end, content[, options] )

Replaces the characters from `start` to `end` with `content`. The same restrictions as `s.remove()` apply. Returns `this`.
Replaces the characters from `start` to `end` with `content`, along with the appended/prepended content in that range. The same restrictions as `s.remove()` apply. Returns `this`.

The fourth argument is optional. It can have a `storeName` property — if `true`, the original name will be stored for later inclusion in a sourcemap's `names` array — and a `contentOnly` property which determines whether only the content is overwritten, or anything that was appended/prepended to the range as well.

It may be preferred to use `s.update(...)` instead if you wish to avoid overwriting the appended/prepended content.

### s.prepend( content )

Prepends the string with the specified content. Returns `this`.
Expand Down Expand Up @@ -220,9 +226,13 @@ Trims content matching `charType` (defaults to `\s`, i.e. whitespace) from the e

Removes empty lines from the start and end. Returns `this`.

### s.isEmpty()
### s.update( start, end, content[, options] )

Returns true if the resulting source is empty (disregarding white space).
Replaces the characters from `start` to `end` with `content`. The same restrictions as `s.remove()` apply. Returns `this`.

The fourth argument is optional. It can have a `storeName` property — if `true`, the original name will be stored for later inclusion in a sourcemap's `names` array — and an `overwrite` property which defaults to `false` and determines whether anything that was appended/prepended to the range will be overwritten along with the original content.

`s.update(start, end, content)` is equivalent to `s.overwrite(start, end, content, { contentOnly: true })`.

## Bundling

Expand Down
18 changes: 17 additions & 1 deletion index.d.ts
Expand Up @@ -98,6 +98,11 @@ export interface OverwriteOptions {
contentOnly?: boolean;
}

export interface UpdateOptions {
storeName?: boolean;
overwrite?: boolean;
}

export default class MagicString {
constructor(str: string, options?: MagicStringOptions);
/**
Expand Down Expand Up @@ -155,13 +160,24 @@ export default class MagicString {
*/
move(start: number, end: number, index: number): MagicString;
/**
* Replaces the characters from `start` to `end` with `content`. The same restrictions as `s.remove()` apply.
* Replaces the characters from `start` to `end` with `content`, along with the appended/prepended content in
* that range. The same restrictions as `s.remove()` apply.
*
* The fourth argument is optional. It can have a storeName property — if true, the original name will be stored
* for later inclusion in a sourcemap's names array — and a contentOnly property which determines whether only
* the content is overwritten, or anything that was appended/prepended to the range as well.
*
* It may be preferred to use `s.update(...)` instead if you wish to avoid overwriting the appended/prepended content.
*/
overwrite(start: number, end: number, content: string, options?: boolean | OverwriteOptions): MagicString;
/**
* Replaces the characters from `start` to `end` with `content`. The same restrictions as `s.remove()` apply.
*
* The fourth argument is optional. It can have a storeName property — if true, the original name will be stored
* for later inclusion in a sourcemap's names array — and an overwrite property which determines whether only
* the content is overwritten, or anything that was appended/prepended to the range as well.
*/
update(start: number, end: number, content: string, options?: boolean | UpdateOptions): MagicString;
/**
* Prepends the string with the specified content.
*/
Expand Down
2 changes: 1 addition & 1 deletion package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 8 additions & 2 deletions src/MagicString.js
Expand Up @@ -349,6 +349,12 @@ export default class MagicString {
}

overwrite(start, end, content, options) {
options = options || {};
options.overwrite = !options.contentOnly;
Copy link

Choose a reason for hiding this comment

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

Modifying a user provided options object isn't great DX and unexpected.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sure, PR welcome

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, you're right. I've sent a PR: #227

return this.update(start, end, content, options);
}

update(start, end, content, options) {
if (typeof content !== 'string') throw new TypeError('replacement content must be a string');

while (start < 0) start += this.original.length;
Expand Down Expand Up @@ -376,7 +382,7 @@ export default class MagicString {
options = { storeName: true };
}
const storeName = options !== undefined ? options.storeName : false;
const contentOnly = options !== undefined ? options.contentOnly : false;
const overwrite = options !== undefined ? options.overwrite : false;

if (storeName) {
const original = this.original.slice(start, end);
Expand All @@ -400,7 +406,7 @@ export default class MagicString {
chunk.edit('', false);
}

first.edit(content, storeName, contentOnly);
first.edit(content, storeName, !overwrite);
} else {
// must be inserting at the end
const newChunk = new Chunk(start, end, '').edit(content, storeName);
Expand Down
135 changes: 135 additions & 0 deletions test/MagicString.js
Expand Up @@ -856,6 +856,141 @@ describe('MagicString', () => {
});
});

describe('update', () => {
it('should replace characters', () => {
const s = new MagicString('abcdefghijkl');

s.update(5, 8, 'FGH');
assert.equal(s.toString(), 'abcdeFGHijkl');
});

it('should throw an error if overlapping replacements are attempted', () => {
const s = new MagicString('abcdefghijkl');

s.update(7, 11, 'xx');

assert.throws(() => s.update(8, 12, 'yy'), /Cannot split a chunk that has already been edited/);

assert.equal(s.toString(), 'abcdefgxxl');

s.update(6, 12, 'yes');
assert.equal(s.toString(), 'abcdefyes');
});

it('should allow contiguous but non-overlapping replacements', () => {
const s = new MagicString('abcdefghijkl');

s.update(3, 6, 'DEF');
assert.equal(s.toString(), 'abcDEFghijkl');

s.update(6, 9, 'GHI');
assert.equal(s.toString(), 'abcDEFGHIjkl');

s.update(0, 3, 'ABC');
assert.equal(s.toString(), 'ABCDEFGHIjkl');

s.update(9, 12, 'JKL');
assert.equal(s.toString(), 'ABCDEFGHIJKL');
});

it('does not replace zero-length inserts at update start location', () => {
const s = new MagicString('abcdefghijkl');

s.remove(0, 6);
s.appendLeft(6, 'DEF');
s.update(6, 9, 'GHI');
assert.equal(s.toString(), 'DEFGHIjkl');
});

it('replaces zero-length inserts inside update with overwrite option', () => {
const s = new MagicString('abcdefghijkl');

s.appendLeft(6, 'XXX');
s.update(3, 9, 'DEFGHI', { overwrite: true });
assert.equal(s.toString(), 'abcDEFGHIjkl');
});

it('replaces non-zero-length inserts inside update', () => {
const s = new MagicString('abcdefghijkl');

s.update(3, 4, 'XXX');
s.update(3, 5, 'DE');
assert.equal(s.toString(), 'abcDEfghijkl');

s.update(7, 8, 'YYY');
s.update(6, 8, 'GH');
assert.equal(s.toString(), 'abcDEfGHijkl');
});

it('should return this', () => {
const s = new MagicString('abcdefghijkl');
assert.strictEqual(s.update(3, 4, 'D'), s);
});

it('should disallow updating zero-length ranges', () => {
const s = new MagicString('x');
assert.throws(() => s.update(0, 0, 'anything'), /Cannot overwrite a zero-length range – use appendLeft or prependRight instead/);
});

it('should throw when given non-string content', () => {
const s = new MagicString('');
assert.throws(() => s.update(0, 1, []), TypeError);
});

it('replaces interior inserts with overwrite option', () => {
const s = new MagicString('abcdefghijkl');

s.appendLeft(1, '&');
s.prependRight(1, '^');
s.appendLeft(3, '!');
s.prependRight(3, '?');
s.update(1, 3, '...', { overwrite: true });
assert.equal(s.toString(), 'a&...?defghijkl');
});

it('preserves interior inserts with `contentOnly: true`', () => {
const s = new MagicString('abcdefghijkl');

s.appendLeft(1, '&');
s.prependRight(1, '^');
s.appendLeft(3, '!');
s.prependRight(3, '?');
s.update(1, 3, '...', { contentOnly: true });
assert.equal(s.toString(), 'a&^...!?defghijkl');
});

it('disallows overwriting partially overlapping moved content', () => {
const s = new MagicString('abcdefghijkl');

s.move(6, 9, 3);
assert.throws(() => s.update(5, 7, 'XX'), /Cannot overwrite across a split point/);
});

it('disallows overwriting fully surrounding content moved away', () => {
const s = new MagicString('abcdefghijkl');

s.move(6, 9, 3);
assert.throws(() => s.update(4, 11, 'XX'), /Cannot overwrite across a split point/);
});

it('disallows overwriting fully surrounding content moved away even if there is another split', () => {
const s = new MagicString('abcdefghijkl');

s.move(6, 9, 3);
s.appendLeft(5, 'foo');
assert.throws(() => s.update(4, 11, 'XX'), /Cannot overwrite across a split point/);
});

it('allows later insertions at the end with overwrite option', () => {
const s = new MagicString('abcdefg');

s.appendLeft(4, '(');
s.update(2, 7, '', { overwrite: true });
s.appendLeft(7, 'h');
assert.equal(s.toString(), 'abh');
});
});

describe('prepend', () => {
it('should prepend content', () => {
const s = new MagicString('abcdefghijkl');
Expand Down