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

Fix webpack usage #59

Closed
trodrigues opened this issue Feb 24, 2016 · 13 comments
Closed

Fix webpack usage #59

trodrigues opened this issue Feb 24, 2016 · 13 comments

Comments

@trodrigues
Copy link
Contributor

The npm package only bundles the built files under dist, but webpack doesn't like the fact that there is a require for a lib dir that doesn't exist.

This can be fixed by including lib but there should be a better way to make webpack not worry about this.

@zamiang
Copy link
Contributor

zamiang commented Jun 2, 2016

Hey @trodrigues I'm having this problem now. How did you end up resolving this issue?

@trodrigues
Copy link
Contributor Author

Hi there,

yes, I did fix this at the time, can't remember on which specific version for sure. Which version are you using at the moment?

@zamiang
Copy link
Contributor

zamiang commented Jun 2, 2016

Running the latest contentful (3.3.6) in a webpack + babel project.

It appears that even though the lib require is in a error handler that is never called, webpack still tries to resolve the require. I forked and removed these lines and all is well. https://github.com/contentful/contentful.js/blob/master/index.js#L7-L9

@trodrigues
Copy link
Contributor Author

Ah yes, that part of the code is there for development mode.

The thing is, webpack should honor the "browser" key on package.json and use the browser.js file instead of index.js.

Could this be an issue with your webpack configuration?

If you'd like to check an example of an app we have that is currently using this, you can have a look at https://github.com/contentful/discovery-app-react/blob/master/webpack.config.js

Khaledgarbaya added a commit that referenced this issue Aug 30, 2016
Support building for a server-side target #59
@Koded
Copy link

Koded commented Jan 11, 2017

I think this is still a problem (only for serverside nodejs). I'm just starting to use the contentful library and it's taking too long to make it work with webpack and node.

The initial problem was the one outlined in this ticket:

ERROR in ./~/contentful/index.js
Module not found: Error: Can't resolve './lib/contentful' in '/Users/jon/Workbench/cms/node_modules/contentful'

The quick fix for this is the solution that @zamiang outlined in https://github.com/contentful/contentful.js/blob/master/index.js#L7-L9

You can fix it with webpack, but the solution doesn't reflect the changes from Webpack 1 to version 2. Specially, to tell webpage to use ./browser.js rather than main.js you need to specify:

resolve: {
  ...
  mainFields: ["browser", "main"]
}

so the webpack.config.js outlined by @trodrigues isn't quite right for webpack v2.

This is a bit funny anyway as you are telling node to use ./browser.js in package.json:

"node": "./browser.js"

https://github.com/contentful/contentful.js/blob/master/package.json#L43

When you get this working, you then need to get webpack to rewrite the reference to the browser version of axios (something like - this isn't working for me yet):

plugins: [
  new webpack.NormalModuleReplacementPlugin(/contentful-sdk-core\/vendor-browser\/axios/g, 'contentful-sdk-core/vendor-node/axios.js')
],

All of this is a little longwinded. Given that I just want to use the contentful service, it feels like this should be a bit easier, and the easy thing to do is to fix index.js by removing the reference to ./lib/contentful.

@Khaledgarbaya
Copy link
Contributor

Hi @Koded,
I am planning to clean up the webpack setup and stop verndorizing axios along side with droping the support for node 0.10 in the upcoming days. Sorry for the bad experience you had.
Best,
Khaled

Koded added a commit to Koded/contentful.js that referenced this issue Jan 11, 2017
@Koded
Copy link

Koded commented Jan 11, 2017

Thanks for the quick response @Khaledgarbaya - good to hear you're going to look into the problem.

@Khaledgarbaya
Copy link
Contributor

@Koded
I started 2 PRs to fix this #108
and https://github.com/contentful/contentful-sdk-core it would be nice it you can help me test with your setup.

Just make sure to switch to the branches for contentful.js and content-sdk-core
There is an npm script that will help you install the contentful-sdk-core local version it needs just to be next to the contentful.js library folder.

@Koded
Copy link

Koded commented Jan 23, 2017

Hi @Khaledgarbaya

I've just checked out that PR branch and built and run it on my app and it all works as expected!

@Khaledgarbaya
Copy link
Contributor

@Koded Thanks a lot, I will do more testing and release it, maybe under beta for a while as it will be a major release

@Khaledgarbaya
Copy link
Contributor

@Koded I hope also you bundle size is a bit smaller now right ?

@irisSchaffer
Copy link

irisSchaffer commented Feb 20, 2017

What exactly is necessary to be able to work with these PRs? I tried npm shrinkwraping but I just couldn't get it to work...

Help with this is highly appreciated!

@Khaledgarbaya
Copy link
Contributor

@irisSchaffer, make sure to change the branch to the PR branch also clone the contentful-sdk-core repo and switch to the branch that is used in this PR

on contentful.js directory run npm install then run npm run devdep:build && npm run devdep:install && npm run build and you can run the project.

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

5 participants