threepointone / ratpack

ready. set. go!
197 stars 10 forks source link

Pragma rules fail when query params present #28

Open rreusser opened 7 years ago

rreusser commented 7 years ago

Is it possible that ratpack isn't handling query parameters in loader names correctly? I'm not all that familiar with webpack and am having some trouble setting up a loader. The obvious fix just passes the problem elsewhere so that I'm not quite sure how to fix it.

My ./ratpack/package.json looks like this:

{
  "name": "ratpack-local",
  "dependencies": {
    "brfs": "^1.4.3",
    "transform-loader": "^0.2.3"
  }
}

My test.js looks like this

/* @ratpack {
  rules: [{
    files: '*.js',
    loader: "transform-loader?brfs",
    enforce: "post"
  }]
} */

I get the following error in the ratpack console:

screen shot 2017-01-12 at 16 29 43

If I remove ?brfs from the loader name, then the module lookup seems to succeed, except now I get this in the browser console:

screen shot 2017-01-12 at 16 30 42

I hate to put this on you (everything else is looking great!) but just wanted to check run by you the possibility that it might be a corner case in how ratpack is handling this. 🚀

threepointone commented 7 years ago

hrmm. ok lemme explain what's going on, for you, and posterity's sake.

so, because of some quirks with electron archives and the lot, the loader isn't directly passed into the webpack config, but passed through require.resolve() to get exact paths before using. This means that the ? syntax won't work, unfortunately. That said, I've actually moved away from using the archive format, so have some ideas for making this work. Will leave this issue open until then.

Would you please tell me your usecase? I've included a bunch of loaders by default, so maybe you won't need it. Unless you're exploring a browserify compat codebase?

On a broader note, with the wealth of browserify transforms out there, I should consider supporting them natively. Will ponder :)

rreusser commented 7 years ago

No worries. Your project is great and I definitely don't want to pull it too heavily toward my peculiar use case at the expense of your time! I just saw a twitter survey indicating a strong general leaning toward webpack (selection bias aside). So I'd vote for keeping it in mind but otherwise neglecting it for now.

Yeah my use case is browserify-compat. My projects rely on glslify and brfs by way of plotly.js.

threepointone commented 7 years ago

you're speaking to a fellow browserify-fan here btw :) only seriously got into webpack sometime last year. There's tons of browserify code out there that I'd like to augment with ratpack, so your use case isn't that peculiar.

quick sketch for a possible api

transforms: [ { files: '*.coffee', transform: 'coffeeify' ... } ]
rreusser commented 7 years ago

Hmm. Would you have to do:

/* @ratpack {
  transforms: [ { files: '*.coffee', transform: 'coffeeify' ... } ]
} */
require('coffeeify');

In order to get it to install coffeeify as required by the transform, or is the transform name always identically equal to the module name? If not, feels like this might still depending on the order of operations, though if the file is preprocessed and node modules installed maybe that would work. detective is the browserify-style module that preprocesses and figures out the required modules. Not sure the webpack/babel equivalent though you must already be doing the equivalent. 😄

threepointone commented 7 years ago

more like

/* @ratpack {
  transforms: [ { files: '*.coffee', transform: 'coffeeify' ... } ]
} */
require('script.coffee');
threepointone commented 7 years ago

(you could just use a webpack coffeescript loader for this specific usecase, but consider brfs etc)

rreusser commented 7 years ago

Ah, I meant just to get the transform-containing module (e.g. brfs) installed in the first place.

threepointone commented 7 years ago

I'll share something in a day or so, I hope you'll be up for some beta testing :)

rreusser commented 7 years ago

Of course! Reading the source now so I can help out a bit instead of just making requests :)

threepointone commented 7 years ago

Lol the source is a mess, sorry in advance

threepointone commented 7 years ago

btw, for glsl, you could try https://www.npmjs.com/package/webpack-glsl-loader

/* @ratpack {
  loaders: [ { files: '*.glsl', loader: 'webpack-glsl-loader' } ]
} */
let shader = require('./something.glsl')
// ...

and if you're really adventurous, you could try implementing a webpack loader that calls the brfs transform internally. (note to self - make sure loader works with relative paths).

threepointone commented 7 years ago

oh wait, glslify uses tagged literals, not the same as this loader. nevermind :|