-
-
Notifications
You must be signed in to change notification settings - Fork 95
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
Static imports to aid bundle traversing tools #334
Conversation
index.js
Outdated
/* istanbul ignore else */ | ||
if (typeof module === 'object' && typeof module.exports === 'object') { | ||
module.exports = f(require(deps[0]), require(deps[1]), require(deps[2])); | ||
module.exports = f(require('sanctuary-def'), require('sanctuary-type-classes'), require('sanctuary-type-identifiers')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I expect the linter to complain about the length of this line. If so, let's use this instead:
module.exports = f(require('sanctuary-def'),
require('sanctuary-type-classes'),
require('sanctuary-type-identifiers'));
Something like this too @davidchambers ? |
Also noticed this in the
|
index.js
Outdated
define(deps, f); | ||
define(['sanctuary-def', | ||
'sanctuary-type-classes', | ||
'sanctuary-type-identifiers'], f); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My rule for line wrapping is that expressions at the same level semantically should be at the same level visually. I'd prefer this wrapping:
define(['sanctuary-def',
'sanctuary-type-classes',
'sanctuary-type-identifiers'],
f);
I seem to recall introducing deps
to avoid the above, because f
looked lonely. 😢
@JAForbes, could you make this change and squash your commits while you're at it? I'll then merge this pull request and release v0.12.1. :)
536ff7c
to
ccb9252
Compare
Browserify requires static imports in order to establish a dependency tree.
ccb9252
to
e111f59
Compare
🌳 |
Browserify require static import's in order to establish a dependency tree.
This PR addresses this issue by replacing e.g.
require(deps[0])
withrequire('sanctuary-def')