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

Add(concat): prefix and suffix options (fix #48) #57

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
3 changes: 3 additions & 0 deletions packages/hopp-plugin-babel/.vscode/settings.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
Copy link
Member

Choose a reason for hiding this comment

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

The .vscode folder should not be pushed to git. Remove this & add it to the .gitignore in the root directory.

"git.ignoreLimitWarning": true
}
1 change: 1 addition & 0 deletions packages/hopp-plugin-concat/example/src/tmp.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
console.log("this is opts js")
28 changes: 28 additions & 0 deletions packages/hopp-plugin-concat/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,35 @@ export const config = {

/**
* We don't need to do any real transformation.
*
* suffix and prefix features
* @param suffix || null @param prefix || null
* appending additional suffix and prefix to each data.
Copy link
Member

Choose a reason for hiding this comment

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

This JSDoc comment is incorrect:

  • Each param definition should be on its own line
  • Description should only be before the param comments start

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

So if i add the comment like:

  /**
   * options assign
   *  @param suffix adding string at the end of the each file
   *  @param prefix adding string at the beginning of the first packet
   */
  var opts = Object.assign({}, ctx.args[0] || {})

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The opts i will fix it

*/
export default async (ctx, data) => {
/**
* unexpected num of arguments handling
*/
if (ctx.args.length > 1) {
throw new Error('Unexpected numbers of arguments.')
}

/**
* options assign
*/
var opts = Object.assign({}, ctx.args[0] || {})
Copy link
Member

Choose a reason for hiding this comment

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

This is only necessary if you are going to modify the opts object, which you aren't. Since you are not modifying the opts, you can just do this: const opts = ctx.args[0]

Also avoid using var in ES2015 code. Use let or const.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

If i changed to javascript const opts=ctx.args[0] would be an error of suffix undefined. so i changed javascript const opts =ctx.args[0] || {}

Copy link
Member

Choose a reason for hiding this comment

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

(y)


// suffix
if (opts.suffix) {
Copy link
Member

Choose a reason for hiding this comment

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

Prefix should only be added if this data packet is the first packet (which it will not always be).

Check if it is the first data piece before adding a prefix.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How do i know how many packets are coming, can i modified the ctx which will store a count?

Copy link
Member

Choose a reason for hiding this comment

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

There's no way to know how many packets but you don't need a count. You just need to know what the first one is & what the last one is.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I feel stupid, which props i can check is the first or the last?

data.size += opts.suffix.length
data.body = Buffer.from(opts.suffix + data.body.toString())
Copy link
Member

Choose a reason for hiding this comment

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

Providing a Buffer is unnecessary. You can just provide a string like this:

data.body = opts.suffix + data.body.toString()

}

// prefix
Copy link
Member

Choose a reason for hiding this comment

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

Prefix should be before data and suffix should be after.

if (opts.prefix) {
Copy link
Member

Choose a reason for hiding this comment

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

Suffix should only be added if data.end is true (i.e. it is the last piece of data)

data.size += opts.prefix.length
data.body = Buffer.from(data.body.toString() + opts.prefix)
}

return data
}