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

Error is wrongly shown as "plugin missing" #529

Closed
matteofigus opened this issue Jul 5, 2017 · 0 comments
Closed

Error is wrongly shown as "plugin missing" #529

matteofigus opened this issue Jul 5, 2017 · 0 comments
Labels

Comments

@matteofigus
Copy link
Member

matteofigus commented Jul 5, 2017

The changes for the plugin detector introduced here unfortunately don't work with common cases.

Example of some components we have in production:

module.exports.data = (cx, cb) => {
        const something = cx.plugins;
        const a = something.discover('something');
        const b = cx.plugins['log']('hello');
        cb(null, { a, b });
};

This is what we get as a failing test:

AssertionError: expected [ Array(2) ] to deeply equal [ 'discover', 'log' ]
      + expected - actual

       [
      -  "\n        const a = something.discover"
      -  "'log']"
      +  "discover"
      +  "log"
       ]

After discussing with @nickbalestra we found out this is not probably the ideal workflow.
This is what we currently have:

  1. User has to include the used plugins inside the package.json
  2. There is no check during the packaging about that
  3. When the registry executes the server.js, there is a preliminary check for declared plugins and registry supported plugins. If the registry doesn't have the plugin, error 501.
  4. If any plugin is not in the list but it's a valid plugin, no error will be raised and plugin executed.
  5. If the execution has an error, the registry will try to run the plugin detective, and if it will match something that is not supported by the registry, will return 501.

We decided that we will change this to:

  1. Parse the server.js in the packaging level, properly done as babel or webpack plugin by actually parsing the code, walking through the AST, and finding the real calls to that methods and comparing across package.json declared plugins (To be done in a future PR inside the template modules) - and fail the compilation if any call to an undeclared plugin is made
  2. On a registry level, we keep the check for unsupported plugins (previous step 3)
  3. On a registry level, when we have an error, instead of trying to parse the code (which is anyways possibly expensive) we will return a generic 500 error (we will rely on webpack on step 1 to ensure this won't possibly be because of a plugin conflict).

To be clear, we are tackling this now, as we found it is blocking us to upgrade our registry to the latest version (some e2e tests are failing because of this bug).

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

No branches or pull requests

1 participant