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

Feature/readme update #85

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Feature/readme update #85

wants to merge 4 commits into from

Conversation

szymon-krysztofiak-dev
Copy link

Configuration for Webpack 4.

@codecov
Copy link

codecov bot commented Apr 21, 2019

Codecov Report

Merging #85 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #85   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           5      5           
  Lines          45     45           
=====================================
  Hits           45     45

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 716a0d6...6095f61. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Apr 21, 2019

Codecov Report

Merging #85 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #85   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           5      5           
  Lines          45     45           
=====================================
  Hits           45     45

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 716a0d6...6095f61. Read the comment docs.

@codecov
Copy link

codecov bot commented Apr 21, 2019

Codecov Report

Merging #85 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #85   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           5      5           
  Lines          45     45           
=====================================
  Hits           45     45

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 776bd43...f8a2244. Read the comment docs.

@fatboypunk
Copy link
Contributor

Thanks for your pull request, looks good to me, @sn3p any thoughts?

@sn3p
Copy link
Collaborator

sn3p commented Apr 29, 2019

@SzymonKrysztofiak thanks for the PR 💯 Will look into it today

@szymon-krysztofiak-dev
Copy link
Author

No problem, btw. this is very good lib, and if You have any suggestions to this PR, i'll find time to look at this again :)

@sn3p
Copy link
Collaborator

sn3p commented Apr 30, 2019

Thanks for sharing your implementation. I really like the path aliases :)

We are in the process of moving from Brunch to Webpack in our apps. Our approach is slightly different, but I guess the result is the same (?). We are kinda new to Webpack so I'm not sure if we are on the right track.

I'm curious, would you prefer one implementation over the other?

assets/webpack.config.js

const glob = require("glob");

module.exports = {
  entry: {
    app: [].concat(
      glob.sync("./js/**/*.js"),
      glob.sync("../lib/app_web/cells/**/*.js"),
      glob.sync("../lib/app_web/views/**/*.js"),

      glob.sync("./css/app.css"),
      glob.sync("../lib/app_web/cells/**/*.css"),
      glob.sync("../lib/app_web/views/**/*.css")
    )
  }
}

assets/js/app.js

import { Builder as CellBuilder } from "@defacto/cell-js";

CellBuilder.reload();

lib/app_web/cells/avatar/index.js

import { Cell, Builder } from "@defacto/cell-js";

class AvatarCell extends Cell {
  initialize() {}
}

Builder.register(AvatarCell, "AvatarCell");

export default AvatarCell;

@sn3p
Copy link
Collaborator

sn3p commented Apr 30, 2019

We are kinda new to Webpack so I'm not sure if we are on the right track.

I'm saying this because we are experiencing some issues with our implementation of CSS Modules (see this post on StackOverflow).

Are you using CSS Modules by any chance?


...
import Cell from "@vendor/cell/cell"
import Builder from "@vendor/cell/builder"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How did you install cell-js? I would expect this:

import { Cell, Builder } from "@defacto/cell-js";

Would you mind sharing your webpack entry config?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I'll respond for couple hours :)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In assets i have src folder and app.js file which is the entry point of my webpack.config.js

  entry: {
    "./js/app.js": "./src/js/app.js"
  },

But now i see that i have a copy of builder.js and cell.js lib in vendor folder. I'll try import it from node modules.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But now i see that i have a copy of builder.js and cell.js lib in vendor folder. I'll try import it from node modules.

That explains 😄
Let me know if importing the package works fo you. If so I'd say we use this in the examples so we can get this merged 🚀

@szymon-krysztofiak-dev
Copy link
Author

Unfortunately, I don't work with CSS-Modules.

Maybe @TheLarkInn will be able to help, he is an expert from Webpack.

@sn3p
Copy link
Collaborator

sn3p commented May 2, 2019

Unfortunately, I don't work with CSS-Modules.

Maybe @TheLarkInn will be able to help, he is an expert from Webpack

Any help or advise is welcome 👍

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

Successfully merging this pull request may close these issues.

3 participants