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

[Plugins] Add separating semicolon when bundling plugins' index.js #1931

Closed
rasteiner opened this issue Nov 25, 2018 · 3 comments

Comments

@rasteiner
Copy link

commented Nov 25, 2018

given the 2 following plugins:

first:

panel.plugin('bla/blah', {
  //stuff
})

second (immediately invoked function expression - result of a build step)

(function() {
  panel.plugin('foo/bar', {
    //stuff
  })
)()

both are valid plugins when installed alone.

When installed together, they become:

panel.plugin(/**/)
(function() { /**/ })()

which javascript interprets as if the return value of panel.plugin(/**/) should be a function (because it's followed by parentheses), therefore it throws an error like undefined is not a function.

It's kinda the fault of plugin author 1, he should have ended his plugin with a semicolon. But in JS semicolons are optional, and it's only a problem because of the concatenation Kirby does.

It would therefore be kinda nice if Kirby would add that missing semicolon when bundling.
It could either just blindly append one (at worst, if there is already one, it becomes ;; which does no harm), or check if the final semicolon is missing (the plugins code seems to be loaded into memory anyway).

@distantnative distantnative changed the title Separate single plugins in index.js bundle by semicolon [Plugins] Add separating semicolon when bundling plugins' index.js Feb 2, 2019

@bnomei

This comment has been minimized.

Copy link

commented Jul 19, 2019

just to make this easier to find via search. a plugin causing this issue might raise the error The field type "XXX" does not exist on another plugin!

@bnomei

This comment has been minimized.

Copy link

commented Jul 19, 2019

i agree with @rasteiner that the panel should do that sanity check before loading a plugin that might cause trouble.

@bastianallgeier

This comment has been minimized.

Copy link
Contributor

commented Aug 19, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
You can’t perform that action at this time.