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

[Feature] Expose extra information in plugin-constraints #31

Closed
1 of 2 tasks
bgotink opened this issue Mar 19, 2019 · 10 comments
Closed
1 of 2 tasks

[Feature] Expose extra information in plugin-constraints #31

bgotink opened this issue Mar 19, 2019 · 10 comments
Labels
enhancement New feature or request

Comments

@bgotink
Copy link
Member

bgotink commented Mar 19, 2019

  • I'd be willing to implement this feature
  • This feature can already be implemented through a plugin

Describe the user story

As a user I want to write constraints while hardcoding as little as possible in the constraints file.

Describe the solution you'd like

Introduce more predicates for the constraints file to consume. I can think of two new useful predicates right now:

  • workspace_private/1 gives the constraints file access to the private property of the workspace.
    Use case: I've got a large monorepo filled with workspaces that use ^ in their dependencies and peerDependencies to allow non-breaking updates of these packages. However, this repo also contains demo packages that explicitly pin their versions. My constraints file should be able to handle that without me having to hardcode a list of all private packages.
  • workspace_root/1 gives the constraints file access to the root workspace's identifier. This allows for accessing dependencies of the root package without having to hard-code what the root package's location or name is.
    Use case: I want to write a rule that enforces all packages to use the same dependency versions as the root package.

Describe the drawbacks of your solution

We don't want to litter the constraints with too many predicates, so we should think hard about the usefulness of every added predicate.

Describe alternatives you've considered

Hardcoding the names of private packages or the root package. This is prone to error, as new private packages can be added or renamed and public packages could be made private and vice versa.

@arcanis
Copy link
Member

arcanis commented Mar 19, 2019

We don't want to litter the constraints with too many predicates, so we should think hard about the usefulness of every added predicate.

I think this is spot on. The main reason why I didn't add predicates for all the fields was that I wasn't too sure about the performance impact of a fact database getting too large 🤔

Assuming it doesn't have performance impact, I say why not. Otherwise, maybe something we could do would be to add a .yarnrc settings enabling some fields:

constraints-fields:
  - private
  - description

The settings above would generate the facts for workspace_private/2 and workspace_description/2.

workspace_root/1 gives the constraints file access to the root workspace's identifier. This allows for accessing dependencies of the root package without having to hard-code what the root package's location or name is.

I believe a better alternative would be to change the workspaces idents to be relativeCwd rather than cwd. This way the root workspace would always be defined via . (and it would in general become easy to reference workspaces by their location). What do you think?

@arcanis
Copy link
Member

arcanis commented Mar 19, 2019

Another option I'm thinking of would be to open an issue on the tau-prolog repository to ask whether they would consider implementing JS-side predicates. This way we would just be able to implement those per-field facts by querying the actual Project instance at runtime (rather than hardcoding each fact one by one). I have no idea how performant or hard it would be, though.

@bgotink
Copy link
Member Author

bgotink commented Mar 22, 2019

I can give you some rough timings for using the package-constraints package. How the prolog rules are generated differs, but the prolog itself is about the same.

  • Project with 20 workspaces with a combined 288 (peer/dev/regular) dependencies, validating a total of 253 matches to gen_enforced_dependency_range: 3.56 seconds
  • Project with 70 workspaces with a combined 1341 (peer/dev/regular) dependencies, validating a total of 1557 matches to gen_enforced_dependency_range: 31.51 seconds

These are the average of three measurements of the tau-prolog querying during normal system load on a 2.9GHz i7 2017 MacBook Pro.

It's already possible to create predicates in javascript using a library module. The downside is you'd need to import the module. Example of how to use the builtin lists module here and the lists module itself is found here. The API is undocumented though, so it'd still be a good idea to open a ticket asking whether they'd consider that part of the API public, and if so, whether any documentation is available or can be made available.

I'm not sure about the performance impact but I'm willing to toy with it and find out.

@arcanis
Copy link
Member

arcanis commented Mar 23, 2019

Oh that's interesting! 😮 31s is significant but I guess it should be ok at least for the first iteration.

(Much) later down the road it might make sense to see whether this could be moved to a partial wasm implementation, like we already do for the libzip.

The downside is you'd need to import the module.

We could do that in the autogenerated header so that by default you're always in a state where workspace_field/3 is available. So something like this would prevent depending on private packages:

gen_enforced_dependency_range(_, DependencyIdent, null) :-
  workspace_ident(WorkspaceCwd, DependencyIdent),
  workspace_field(WorkspaceCwd, 'private', true).

@bgotink
Copy link
Member Author

bgotink commented Mar 23, 2019

The following might be of interest: SWI-Prolog supports webassembly as of version 8.1.2. This version was released 19 days ago. Note that odd minor releases are considered development builds, not stable releases.

SWI-Prolog is a lot stricter than tau-prolog though: when running the generated full constraint source through their repl I get a lot of warnings about discontiguous predicates. We could disable those warnings or we could generate the predicates in a different order.

@arcanis
Copy link
Member

arcanis commented Mar 23, 2019

Ahah, I've looked for this every few months for the past five years but it's only now that there's a credible JS alternative that they did it 😄 Could be interesting to experiment with this later on.

Regarding workspace_field, would you be ok to send a PR implenting it?

@bgotink
Copy link
Member Author

bgotink commented Mar 23, 2019

Sure! Though I'm not sure what to implement. Do you want to have a PR where I just expose the private field or do you want a more sophisticated setup like the javascript setup or the .yarnrc idea you both mentioned above?

I've been toying with the first idea: a module in javascript that's exposed in tau-prolog as a module.
Source in the package-constraints repo can be found here. There's a significant performance difference: the prolog inference in the workspaces mentioned above now takes respectively 1.312 seconds and 6.106 seconds.

However… running the full constraints through swipl via wasm in chrome yields a runtime of only .144 seconds for the largest of the two workspaces. That's 40 times faster than using a javascript module in tau-prolog.

@arcanis
Copy link
Member

arcanis commented Mar 27, 2019

a more sophisticated setup like the javascript setup or the .yarnrc idea you both mentioned above?

I'd say the second one - having a workspace_field/3 predicate (first implemented as a Javascript module, but then later it might make sense to do something smarter such as pre-parse the user prolog code to figure out which fields are needed and bake them into the fact database). Would it negatively impact the performances of projects that don't use it this function?

@bgotink
Copy link
Member Author

bgotink commented Mar 27, 2019

Given that the function would only need to query the workspaces once it's called I don't think there will be any negative impact. I'll try and get some timings with and without the module while I'm implementing it and include those in the PR description

@arcanis
Copy link
Member

arcanis commented Apr 16, 2019

Going to close this issue now that #53 is merged! And I've opened #68 and #69 for possible followups.

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

No branches or pull requests

2 participants