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

Componentinized lib build #535

Draft
wants to merge 7 commits into
base: develop
Choose a base branch
from
Draft

Conversation

nicksp
Copy link
Collaborator

@nicksp nicksp commented May 6, 2020

THIS IS A DRAFT VERSION AND NOT FINALIZED YET!

The code is also not clean or final. I'd like to get the concept working first and only then focus on optimizations and cleanup.

The idea is to import individual components like: xp-ui/Button rather than the entire library as we do currently. Doing so pulls in only the specific components that we need in each particular component/page, which can significantly reduce the amount of code we end up sending to the client.

Related PRs

branch PR
optimized-component-import XP Registration

If things get working as expected, we might even reduce the number of webpack.config files to one (instead of two that I introduced) and build all the components inside root /lib folder so we can reuse and pull them in inside both xp-registration and xp-api (in other words there won't be a need to keep separate bundles anymore). We have all the components in one place ready to be pulled in independently OR we can still use all.js to pull in components in one go like we already do: import { ComponentA, ComponentB } from 'xp-ui'. This way we support existing xp-api and xp imports and allow for flexibility when working with xp-registration where we expect some components to be lazy loaded.

I've changed the build NPM script command to run custom node script (/scripts/compile.js) to bundle index file (so we can still do import { ComponentA } from 'xp-ui) and babel-ify individual components so we can pull them in individually from the client apps. I might definitely be missing smth as the final setup doesn't work as it should 🤔 But sharing to keep it visible and so that everyone can try that out locally. It's likely that the way we build individual components should be changed somehow, something should be tuned to make this all work 😛 🤔

An alternative to a custom build script (which I find very flexible and a clean way to expose logs into console), I also tried the following command for the build script which does pretty much the same thing (keeping it here for the reference only):

"build": "webpack --mode production && babel ./src/js/ --ignore '__tests__' --out-dir ./dist --copy-files && cross-env BABEL_ENV=es6 babel ./src/js/ --ignore '__tests__' --out-dir ./dist/es6 --copy-files",

Here's the structure we generate in xp-ui with npm run build command:

image

It's assumed that having those files/folders prebuilt and babelified we could reference them from the client apps like xp-registration, so that the import statement looks like that:

import Button from 'xp-ui/lib/registration/components/ui/Button'
import InputField from 'xp-ui/lib/registration/components/forms/InputField'

It's important to run npm i git://github.com/x-team/xp-ui.git#componentinized-lib-build inside xp-registration first to install the required dev-libs I used for development.

On the client side (in xp-registration repo), we'll need to run npm I to install the correct test version of xp-ui and to auto-generate /lib folder. Here's how installed node_modules/xp-ui/ looks like after installing xp-ui:

image

Imports inside xp-registration have also been updated to reflect the new reference path.

Current issues:

  1. npm run build in xp-registration fails with:
    image
  2. seems like we're missing some setup bits that properly bundle individual components to let them behave on their own

@nicksp nicksp self-assigned this May 6, 2020
@nicksp nicksp marked this pull request as draft May 6, 2020 11:56
@nicksp nicksp mentioned this pull request May 8, 2020
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant