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

Map default export to module for es-module interoperability #20

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

tswaters
Copy link

@tswaters tswaters commented Sep 9, 2017

Using es modules (i.e., using @std/esm) doesn't work very well with Sails. Most things that sails magically requires are run through this module and it doesn't account for a default export from an es module.

When using the following syntax in, e.g., ./api/policies/dummy.js:

export default (req, res, next) => next()

Sails will issue the following complaint:

In ${controller} ignored invalid attempt to bind route to a non-function controller: { default: [Function: wrapper],
  identity: 'dummy',
  globalId: 'dummy',
  etc...

And an error will be thrown attempting to call _.bind on {default: [Function: wrapper]} (TypeError: Expected a Function).

This can apply to config files as well.... the ./config/*.js aren't really a problem as they use named exports -- this is more for in ./config/env/*.js which the sails generator create as default exports.

This PR attempts to rectify this globally for sails by mapping module.default to module, if it exists. This will allow for interoperability with es-modules in sails applications.

@tswaters
Copy link
Author

tswaters commented Sep 9, 2017

Hm. Now that I've submitted this PR, I realize any additional exports will be lost, and only default will be used.

I could see this being a problem for services where one might have both a default export and potentially other exports... the case might be a class returned as default export and maybe other utilities.

export default class SomeClass {}
export const UtilFn = () => {}

That use-case might be a bit strained itself; you'll lose prototype hierarchy exporting classes from services.... also to do this with cjs style would require some hackery adding static methods to the class,

module.exports = class SomeClass {}
module.exports.UtilFn = () => {}

Still, converting this to es module one would completely lose any additional exports -- and that doesn't seem correct.

I'm not sure if this is a concern or not... it's workable, but there are edge cases that are weird (i.e. default export of a primitive type, like the test; can't attach additional properties).

Thoughts?

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

Successfully merging this pull request may close these issues.

1 participant