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

Prevent duplication while ejecting stage presets #80

Closed
wants to merge 3 commits into from
Closed

Prevent duplication while ejecting stage presets #80

wants to merge 3 commits into from

Conversation

aulisius
Copy link
Contributor

This is towards closing #77

For the time being, have added the failing test case.

@aulisius aulisius changed the title Add failing test case. Prevent duplication while ejecting stage presets Aug 30, 2018
@@ -61,14 +61,15 @@ function changePresets(config, options = {}) {
}

if (newPlugins.length > 0) {
config.plugins = (config.plugins || []).concat(...newPlugins);
config.plugins = (config.plugins || []);
Copy link
Member

Choose a reason for hiding this comment

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

Since we are not concating here anymore, this if statement should be moved where we actually need to guard against config.plugins from being undefined.


if (config.env) {
Object.keys(config.env).forEach((env) => {
changePresets(config.env[env]);
changePlugins(config.env[env]);
let updatedPlugins = changePresets(config.env[env]);
Copy link
Member

Choose a reason for hiding this comment

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

newPlugins instead of updatedPlugins, since we didn't update the plugins list yet?

@aulisius
Copy link
Contributor Author

PR fixes done! :D

@ljharb ljharb mentioned this pull request Aug 30, 2018
@aulisius
Copy link
Contributor Author

aulisius commented Sep 2, 2018

can we get this merged @nicolo-ribaudo @existentialism ?

@nicolo-ribaudo
Copy link
Member

Does this work with plugins ejected from the stage presets which have options? e.g.

{
  "plugins": [
     "@babel/plugin-proposal-decorators"
  ],
  "presets": [
    "stage-2"
  ]
}

@aulisius
Copy link
Contributor Author

aulisius commented Sep 2, 2018

This is the output that is generated. @nicolo-ribaudo Can you verify if this is the expected output? If this is what we want, will add a test for the same.

Object {
  "plugins": Array [
    Array [
      "@babel/plugin-proposal-decorators",
      Object {
        "legacy": true,
      },
    ],
    "@babel/plugin-syntax-dynamic-import",
    "@babel/plugin-syntax-import-meta",
    "@babel/plugin-proposal-class-properties",
    "@babel/plugin-proposal-json-strings",
    Array [
      "@babel/plugin-proposal-decorators",
      Object {
        "legacy": true,
      },
    ],
    "@babel/plugin-proposal-function-sent",
    "@babel/plugin-proposal-export-namespace-from",
    "@babel/plugin-proposal-numeric-separator",
    "@babel/plugin-proposal-throw-expressions",
  ],
  "presets": Array [],
};

Edit: Just noticed that the output is wrong. Will fix that.

@aulisius
Copy link
Contributor Author

Closing this in favor of #100 which is a better solution than mine.

@aulisius aulisius closed this Oct 24, 2018
@aulisius aulisius deleted the prevent-duplicate-plugins branch October 24, 2018 17:56
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

Successfully merging this pull request may close these issues.

None yet

2 participants