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

How to avoid array concatenation #244

Closed
wpoortman opened this issue Feb 23, 2022 · 18 comments
Closed

How to avoid array concatenation #244

wpoortman opened this issue Feb 23, 2022 · 18 comments

Comments

@wpoortman
Copy link

wpoortman commented Feb 23, 2022

So in my current situation, all the array are concatenated by default like the docs explain. Now I've been trying to write a custom 'arrayMerge' as an option to avoid it and just extend the array instead of gluing them after each other.

Current situation:

deepmerge({
  purge: {
    content: [
      'a/path/to/my/file-a',
      'a/path/to/my/file-b'
    ]
  }
}, {
  purge: {
    content: [
      'a/path/to/my/file-c',
      'a/path/to/my/file-d'
    ]
  }
})

Should output:

{
  purge: {
    content: [
      'a/path/to/my/file-c',
      'a/path/to/my/file-d',
      'a/path/to/my/file-c',
      'a/path/to/my/file-d'
    ]
  }
}

Can't figure this out... any solution?

@RebeccaStevens
Copy link

I'm not sure what you mean. What's the difference between what it does by default and what you want?

@RebeccaStevens
Copy link

Is this what you want?

{
  arrayMerge: (target, source, options) => {
    const destination = source.slice();
    source.forEach((item, index) => {
      destination.push(item);
    });
    return destination;
  }
}

@wpoortman
Copy link
Author

wpoortman commented Feb 23, 2022

I'm not sure what you mean. What's the difference between what it does by default and what you want?

My current deepmerge.all() returns:

{
  purge: {
    content: [
      'a/path/to/my/file-ca/path/to/my/file-da/path/to/my/file-ca/path/to/my/file-d'
    ]
  }
}

Instead of:

{
  purge: {
    content: [
      'a/path/to/my/file-c',
      'a/path/to/my/file-d',
      'a/path/to/my/file-c',
      'a/path/to/my/file-d'
    ]
  }
}

Edit: arrayMerge didn't work, the item is already concatenated when it does destination.push(item)

@RebeccaStevens
Copy link

oh, the strings are getting concatenated together.
What does your arrayMerge function look like?

@wpoortman
Copy link
Author

Currently no options are included, so the default of deepmerge.all()

@RebeccaStevens
Copy link

RebeccaStevens commented Feb 23, 2022

I'm not able to recreate the issue: https://codesandbox.io/s/stupefied-matan-df2ri6?file=/src/index.ts

@RebeccaStevens
Copy link

I'm not sure what might be causing this issue. You could try using my library instead and see if you are having the same issue with it: https://www.npmjs.com/package/deepmerge-ts

Note: deepmerge-ts was originally planned to be the next version of this library.

@wpoortman
Copy link
Author

Which function in your lib would replace the .all() method?

@RebeccaStevens
Copy link

RebeccaStevens commented Feb 23, 2022

deepmerge; it can take any number of arguments.

Note: If you want to provide it with an array of values, call it using the spread operator e.g. deepmerge(...args)

@wpoortman
Copy link
Author

And deepmerge a object with an object? For some reason I can't figure out this API 😄

I have two situations:

  1. Merge a array of objects
  2. Merge an object with the result of 1

Note: Previously I would do deepmerge(deepmerge.all(myArray.map((extension) => {})), MyBaseObject)

@RebeccaStevens
Copy link

So for 1. - Merging an array of objects into a single object, would just be deepmerge(...myArrayOfObjects).

I'm not sure what you mean by 2. but if it is just that line of code, the equivalent would be:
deepmerge(...(myArray.map((extension) => {})), MyBaseObject)

@RebeccaStevens
Copy link

If you link me your code, I can take a look at it for you.

@wpoortman
Copy link
Author

https://pastebin.com/qJ5c1j1c

The themes.json looks like:

{
  "extensions": [
    {
      "name": "package-a",
      "src": "vendor/internal/package-a/src"
    },
    {
      "name": "package-b",
      "src": "vendor/internal/package-b/src"
    }
  ]
}

And a custom tailwind.config.js inside of one of those paths:

module.exports = {
    purge: {
        content: [
            '/view/frontend/templates/**/*.phtml',
        ]
    }
}

Put a lot of time in this... almost at a point where I'm just telling myself to use the fix and move on haha

@RebeccaStevens
Copy link

RebeccaStevens commented Feb 23, 2022

Does this work for you?

return deepmerge(
  ...themeConfig.extensions.map((extension) => {
    const configFilePath = themeDirBackTrails[backtrail] + extension.src + "/tailwind.config.js";

    if (fs.existsSync(configFilePath)) {
      return require(configFilePath);
    }

    return {};
  }),
  baseConfig
);

Here's a playground link: https://codesandbox.io/s/beautiful-perlman-edswlz?file=/src/test.js

@wpoortman
Copy link
Author

It's doing something, but break somewhere in the middle. Hard to explain what's going on.

@RebeccaStevens
Copy link

If you're able to provide more data I'll look into it more tomorrow.

@wpoortman
Copy link
Author

If you're able to provide more data I'll look into it more tomorrow.

Is there a way I can reach you via chat or something? Would communicate a lot easier if you ask me. I'm going with my current solution because I really spent to much time into this issue where I can't understand a package would by default concatenate arrays...

Let me know

@TehShrike
Copy link
Owner

To get more help you would need to provide a REPL link that replicates your problem, and demonstrates the output that you expect as opposed to what the code produces

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

No branches or pull requests

3 participants