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

jquery dependency fix for check package #6710

Closed
wants to merge 5 commits into from
Closed

jquery dependency fix for check package #6710

wants to merge 5 commits into from

Conversation

skirunman
Copy link
Contributor

@skirunman skirunman commented Apr 5, 2016

This is is meant to fix the jquery dependency in the check package for the isPlainObject function by using a version of isPlainObject from lodash, issue #6563

I was not able to get tests to run in my environment and this should be checked by someone with more experience on Meteor core. I hope this can at least help get this fix implemented soon.

@skirunman
Copy link
Contributor Author

I just want to reiterate this is a work in progress and needs to be tested.

@@ -3,6 +3,9 @@
// Things we explicitly do NOT support:
// - heterogenous arrays

//XXX: import from main lodash library if Meteor updates to use lodash instead of underscore
const isPlainObject = Npm.require('lodash.isplainobject');
Copy link
Contributor

Choose a reason for hiding this comment

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

This works also on client? This is new in 1.3? Interesting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think you are right that it does not work on client.

Can we just use require now with 1.3? It looks like modules, ecmascript and babel packages are in core build now.

Like I said, have not tested and just trying to make some progress.

@@ -4,15 +4,17 @@ Package.describe({
});

Package.onUse(function (api) {
api.use(['underscore', 'ejson'], ['client', 'server']);
api.use('jquery', 'client');
api.use(['underscore', 'modules', 'ejson'], ['client', 'server']);
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you want to actually depend on ecmascript instead of modules here, especially if you are using const in the code.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried that, but got an error with a circular reference that ecmascript package depended on check. I'll just change const to var.

@stubailo stubailo self-assigned this Apr 6, 2016
@skirunman
Copy link
Contributor Author

Closing as solution implemented as shim function.

@skirunman skirunman closed this Apr 20, 2016
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

3 participants