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

purescript-affjax: Dynamic require #5

Closed
garyb opened this issue Jan 10, 2017 · 7 comments
Closed

purescript-affjax: Dynamic require #5

garyb opened this issue Jan 10, 2017 · 7 comments

Comments

@garyb
Copy link

garyb commented Jan 10, 2017

There will be several cases of this, as a pattern like this: https://github.com/slamdata/purescript-affjax/blob/071ca67b3535bd20e33b0765251c35a67278d77f/src/Network/HTTP/Affjax.js#L12 is used in a few libraries - whenever a module supports both node and browser and has a shim for one or the other, pretty much.

@Pauan
Copy link
Owner

Pauan commented Jan 10, 2017

The warning just means that it's using CommonJS in a dynamic way, which isn't allowed with ES6 modules.

In the specific case of purescript-affjax, it seems harmless. It's just doing a dynamic check for whether it's on Node.js or not, so that way it can load polyfills.

The problems happen when the code has imports/exports which aren't at the top level. As long as all the imports/exports are at the top level, then it's probably fine, and you can ignore the warnings.

@Pauan Pauan closed this as completed Jan 10, 2017
@garyb
Copy link
Author

garyb commented Jan 10, 2017

We could probably use a comment or something for suppressing these warnings, as I know of at least 3 libraries that use this technique (affjax, debug, websockets-simple).

@Pauan
Copy link
Owner

Pauan commented Jan 10, 2017

I don't think the parser keeps the comments in the AST, though, so I don't think that would be a good solution. Perhaps something similar to "use strict";? Something like "ignore warnings";

@garyb
Copy link
Author

garyb commented Jan 10, 2017

Hmm, that's unfortunate - I wouldn't want to supress all warnings as it could hide other real issues.

@garyb
Copy link
Author

garyb commented Jan 10, 2017

"ignore dynamic require"; maybe? 😉

@Pauan
Copy link
Owner

Pauan commented Jan 10, 2017

I think a comment would work if I used a regexp to parse the comments.

I think the form would be something like // rollup-plugin-purs ignore dynamic require, and similar for module and exports

@Pauan
Copy link
Owner

Pauan commented Jan 10, 2017

Okay, version 1.0.6 added in three comment pragmas:

// rollup-plugin-purs ignore dynamic exports
// rollup-plugin-purs ignore dynamic require
// rollup-plugin-purs ignore dynamic module

The comments must have that exact form, and they must appear at the top-level, without any spaces between the start of the line and the // part of the comment.

Each comment will disable a specific warning.

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

No branches or pull requests

2 participants