Nyalab / caniuse-api

request the caniuse data to check browsers compatibilities
MIT License
356 stars 27 forks source link

Remove the generation step #42

Closed MoOx closed 8 years ago

MoOx commented 8 years ago

It was used to provide compat for browserify, but this cause more problem than it solves. Removing this will fix #29 #32 #36 #39 #40.

And bundler like webpack might be able to handle the dynamic require.

MoOx commented 8 years ago

PR welcome if anyone comes around.

Nyalab commented 8 years ago

yes, it may be time to think about it

tiagocpontesp commented 8 years ago

Can we please get #41 merged in the meantime? Isn't it better than adding a jspm package-override in the grand scheme of things?

ScottKaye commented 8 years ago

Looks like the generation step is completely failing for me because of shelljs:

X:\Scott\Projects\wc1\node_modules\caniuse-api>dir
...
2016-08-08  09:45 PM    <DIR>          .
2016-08-08  09:45 PM    <DIR>          ..
2016-08-08  09:41 PM             1,710 CHANGELOG.md
2016-08-08  09:41 PM    <DIR>          dist
2016-08-08  09:41 PM               193 generator.js
2016-08-08  09:41 PM             1,084 LICENSE
2016-08-08  09:41 PM             2,790 package.json
2016-08-08  09:41 PM             2,656 README.md

X:\Scott\Projects\wc1\node_modules\caniuse-api>node generator
fs.js:1568
  return binding.realpath(pathModule._makeLong(path), options.encoding);
                 ^

Error: EISDIR: illegal operation on a directory, realpath 'R:\Temp\shelljs_4ae1a0f6948005172c00'
    at Error (native)
    at Object.realpathSync (fs.js:1568:18)
    at Function.Module._findPath (module.js:167:25)
    at Function.Module._resolveFilename (module.js:438:25)
    at Function.Module._load (module.js:388:25)
    at Module.runMain (module.js:575:10)
    at run (node.js:348:7)
    at startup (node.js:140:9)
    at node.js:463:3

caniuse-api: Generation ok

Might be because my temp directory is on a different drive. Executing node dist/generate-features.js generates features.js just fine though.

Replacing the entirety of generator.js with this solves the issue for me:

require("child_process").exec("node dist/generate-features.js");

I can put this in a PR if this works for you guys - it's my bandaid solution, but looks like it works properly to me.

alexisvincent commented 8 years ago

Can this be implemented?

MoOx commented 8 years ago

Yes sure. https://github.com/Nyalab/caniuse-api/issues/42#issuecomment-218408770

alexisvincent commented 8 years ago

@MoOx What would happen if we just moved the generation step from post install to build

alexisvincent commented 8 years ago

@MoOx Ok, so from what I can see, you want to read the local db. Would you be happy if i submitted a pr that performed the generation step when the file is required?

MoOx commented 8 years ago

We should just remove it and don't care about browserify and stuff. We might add a solution for browserify and friends to accept some hardcoded data.

MoOx commented 8 years ago

(because this step was basically to avoid dynamic require() call so browserify can handle it).

alexisvincent commented 8 years ago

Oh i see, ok, how would you suggest going about this?

MoOx commented 8 years ago

For now just try to remove this generation step and try to access directly require files when needed?

alexisvincent commented 8 years ago

The problem that I'm hitting is with jspm and cssnext (which relies on this). So anyway the postInstall step is not being run. So when cssnext wants to use the api, this project internally accesses the features.js file. Which hasn't yet been created. So at some point we actually need to get the features file. Unless what you're saying is to modify this project so that it directly requires the features file from db?

MoOx commented 8 years ago

Unless what you're saying is to modify this project so that it directly requires the features file from db?

That's what I am saying :)

MoOx commented 8 years ago

Closed by #47