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

Remove the generation step #42

Closed
MoOx opened this issue May 11, 2016 · 15 comments
Closed

Remove the generation step #42

MoOx opened this issue May 11, 2016 · 15 comments

Comments

@MoOx
Copy link
Collaborator

MoOx commented May 11, 2016

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
Copy link
Collaborator Author

MoOx commented May 11, 2016

PR welcome if anyone comes around.

@Nyalab
Copy link
Owner

Nyalab commented May 12, 2016

yes, it may be time to think about it

@tiagocpontesp
Copy link
Contributor

tiagocpontesp commented May 30, 2016

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
Copy link

ScottKaye commented Aug 9, 2016

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
Copy link
Contributor

Can this be implemented?

@MoOx
Copy link
Collaborator Author

MoOx commented Aug 26, 2016

Yes sure. #42 (comment)

@alexisvincent
Copy link
Contributor

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

@alexisvincent
Copy link
Contributor

@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
Copy link
Collaborator Author

MoOx commented Aug 26, 2016

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
Copy link
Collaborator Author

MoOx commented Aug 26, 2016

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

@alexisvincent
Copy link
Contributor

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

@MoOx
Copy link
Collaborator Author

MoOx commented Aug 26, 2016

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

@alexisvincent
Copy link
Contributor

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
Copy link
Collaborator Author

MoOx commented Aug 26, 2016

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
Copy link
Collaborator Author

MoOx commented Sep 5, 2016

Closed by #47

@MoOx MoOx closed this as completed Sep 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants