Skip to content
This repository has been archived by the owner on Aug 4, 2021. It is now read-only.

Make v2 compatible with v1 #58

Closed
thgh opened this issue Sep 17, 2019 · 4 comments · Fixed by #61
Closed

Make v2 compatible with v1 #58

thgh opened this issue Sep 17, 2019 · 4 comments · Fixed by #61

Comments

@thgh
Copy link

thgh commented Sep 17, 2019

V2 seems to have removed this syntax: (PR #53)

alias({
  src: __dirname + '/src'
}),

I would suggest to revert this change and add special behaviour for the entries option if it's an array.

@thiscantbeserious
Copy link
Contributor

thiscantbeserious commented Sep 17, 2019

In reference to your comment and my response here ...

#53 (comment)

I checked MDN if there's a way - and indeed Object.entries does return it in the correct way (converting it to an Array) as defined - and actually enables us to support the old config without to much change to the current code-base.

Maybe we really want to go back to the V1 config-syntax just for the sake of keeping things simple and questions at a minimum (even tought it promotes laziness and is kind of dirty in terms of coding standards), stick to the current config with the entries property (V2), a mixture of both removing just the find and replacement properties - or really provide both options.

I'd leave it up to @lukastaegert to decide before doing a PR.

@lukastaegert
Copy link
Member

From an API perspective, I see no reason not to support both syntaxes. The plugin could decide depending on the presence of the entries key which syntax is used. This would make the plugin a little more complex but in essence what could happen internally is that if entries is not supplied, the plugin does a best guess as to what entries should be—in that case, we assume the user does not need or care about ordering. Possibly use Object.keys rather than Object.entries as the latter is only supported on latest Node.

Apparently there is some demand for the concise old syntax and of course supporting it again would mean that people do not need a migration.

@lukastaegert
Copy link
Member

I would not want to remove the new syntax again as it has been out for some time and it would be REALLY nasty to alienate early adopters. Also, the new syntax is more expressive.

@thiscantbeserious
Copy link
Contributor

thiscantbeserious commented Sep 18, 2019

Alright, also agreed on Object.keys. I'll see what I can do and will submit an PR later on.

Update: Andarist was faster then me with PR #61 - I've did a code-review instead 😄

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

Successfully merging a pull request may close this issue.

3 participants