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

Override any package.json key in overrides #1865

Closed
itaymendel opened this issue Jul 28, 2019 · 10 comments
Closed

Override any package.json key in overrides #1865

itaymendel opened this issue Jul 28, 2019 · 10 comments

Comments

@itaymendel
Copy link
Contributor

Description

A developer should have better control over the component's package.json and its contents. At the moment the ability to so is limited due to the fact that overrides only allows for modifications on the dependencies, peerDependencies and devDependncies fields. So if a developer wants to have a bin for the component - it is not possible.
This hurts the component's flexibility.

Describe the solution you'd like

overrides should allow setting ANY key/value pair in the component's package.json. Bit should then take all values and set them to the calculated package.json.

Blacklisted keys

There are some keys that the developer must not be able to override:

  • name
  • version
  • main

Input validation to package.json keys

Bit has to make sure that the calculated package.json will be a valid one. Meaning that if a specific key is used (for example bin) - its type should be according to the specs (perhaps using something like this?).
In case the outcome for a component is not valid, Bit should not allow tagging the component and prompt with a message saying that the key is not set according to the package.json specs.

Merging values of similar keys

The overrides feature supports merging for dependencies, peerDependencies and devDependencies in case that the same component fit more than one overriding rule. Bit should keep this behavior (and still ensure that the more specific rule wins).

In cases where a merge is not possible (the value in either override rules is not of the same type) - the more specific rule should "win".

Adding keys outside of the package.json specs

Bit should allow developers to add any key they want, even if its not in the specs, as long as the output is still a valid json file. Additionally, all merge rules should support it as well.

@davidfirst
Copy link
Member

davidfirst commented Aug 2, 2019

Implemented on #1890 as described above.
Added e2e-tests and unit tests for different scenarios.
The validation is done by the API of https://github.com/gorillamania/package.json-validator as suggested.
To the blacklisted keys I have added also bit.
The merge between a more general and a more specific rule is done only when both are objects. Otherwise, the specific value wins.

davidfirst added a commit that referenced this issue Aug 2, 2019
…re (#1890)

* validate package.json values entered by users against npm specs

* merge package.json props from a more general rule to a specific one
@itaymendel
Copy link
Contributor Author

@KutnerUri @qballer now we can implement benv as a component :))))))))

@KutnerUri
Copy link
Contributor

@itaymendel - so cool!!
where are the docs for it?

@itaymendel
Copy link
Contributor Author

itaymendel commented Aug 4, 2019

it's not yet GA, so docs are not public... But it should look like this (in your project's package.josn):

"bit": {
    "overrides": {
        "<component id>": {
            "bin": /* your bin object here */
        }
    }
}

If you want to play with it, you should clone Bit and build from master.

@KutnerUri
Copy link
Contributor

SOO NICE!

@Tallyb
Copy link
Contributor

Tallyb commented Sep 12, 2019

@itaymendel @GiladShoham: @davidfirst probably wiser would be to wrap in an object to avoid collisions with npm attributes. E.g. if npm decide to add "exclude" as a valid package property.

@GiladShoham
Copy link
Member

@Tallyb but we don't want to wrap it because there are features on the root keys.
for example, bin will only work on the root.

@Tallyb
Copy link
Contributor

Tallyb commented Sep 12, 2019

overrides: {
"someRule/*": {
   "env": {...},
   "devDeps": {},
   "exclude": true,
   "package": {
        "bin": "...",
       "something": {}
}
}
}

alternatively, you should put all internal variables (propagate, exclude) on some

"_options": { ...}

@GiladShoham
Copy link
Member

@Tallyb. Ok understand. Make a total sense.. We will change it.

@GiladShoham
Copy link
Member

@Tallyb Can you open a new issue for this improvement? if we keep it here we will forget it.

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

No branches or pull requests

5 participants