Skip to content

UIKit Refactor: Bower to NPM Migration, Partial JS Refactor, and New JS Build Pipeline #915

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

Merged

Conversation

sghoweri
Copy link
Contributor

@sghoweri sghoweri commented Jul 29, 2018

Here’s the big one I’ve been waiting a long, long time to finally get into PL core 🎉

There’s a bunch of small, under the hood changes here (see commits for the specifics) but in a nutshell:

  1. Migrate Pattern Lab’s UIKit away from Bower and fully onto NPM
  2. Reorganize UIKit’s JavaScript into 3 main buckets to help with separating out the different concerns: the main JS entrypoints (which didn’t previously exist as physical files, based on how things had been set up), components, and utils.
  3. Painstakingly go through every single JS file in PL and make sure every file properly pulls in and manages their own dependencies required
  4. Fix a TON of broken JS code — undefined or improperly scoped variables and the like
  5. Adds a very minimal Webpack build to bundle and minify the JS.

Lots more to come based off of this work but really just trying to start small to things keep things in working order + get us in a good spot to start working on some much more abitious work on the UI side of things!

Would love any help I can get to further test this out to make sure all the kinks and gotchas have been ironed out!

CC @pattern-lab/devs

sghoweri added 26 commits July 28, 2018 21:34
…nger maintained jwerty library for very comparable Mousetrap library
… how the viewport panel gets resized -- saving for a separate PR to limit scope of changes
…ned variables; refactor, clean up logic that opens/closes the active code panel
…ring panel animation to shift to using CSS vars + disable transitions temporarily when animating
…ic check to make sure typeahead's JS only gets initialized once
…config that had been using Bower + script tags in the HTML
… JS logic that had been coming in from Bower + hard-coded HTML logic
@sghoweri
Copy link
Contributor Author

By the way @bmuenzenmeyer, before you bring it up, I’d really love to run all these JS files through Prettier — I just wanted to wait till the overall code changes here were understood and OK’d first so we aren’t looking at 10x the number of JS changes 😉

@bmuenzenmeyer
Copy link
Member

Love it. Will provide a more comprehensive review ASAP.

@bmuenzenmeyer bmuenzenmeyer self-requested a review August 3, 2018 14:59
@bmuenzenmeyer
Copy link
Member

@sghoweri this is knock-down drag-out awesome work. Thank you so much for putting so much time and energy into it!

Am I testing this correctly:

from your branch...

$ npm install && npm run bootstrap
$ cd packages/uikit-workshop
$ npm run build

> @pattern-lab/uikit-workshop@1.0.0-alpha.7 build C:\src\patternlab-node-pr\packages\uikit-workshop
> gulp && npx webpack-cli --config webpack.config.js --progress

[10:27:52] Using gulpfile C:\src\patternlab-node-pr\packages\uikit-workshop\gulpfile.js[10:27:52] Starting 'build:css'...
[10:27:53] Starting 'clean:html'...
[10:27:53] Finished 'clean:html' after 46 ms
[10:27:53] Starting 'build:html'...
[10:27:53] Finished 'build:html' after 250 ms
Autoprefixer's process() method is deprecated and will removed in next major release. Use postcss([autoprefixer]).process() instead
[10:27:54] Finished 'build:css' after 1.99 s
[10:27:54] Starting 'default'...
[10:27:54] Finished 'default' after 58 μs
npx: installed 111 in 20.417s
Cannot find module 'webpack'
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! @pattern-lab/uikit-workshop@1.0.0-alpha.7 build: `gulp && npx webpack-cli --config webpack.config.js --progress`
npm ERR! Exit status 1

@sghoweri
Copy link
Contributor Author

sghoweri commented Aug 3, 2018

Hmm 🤔 that sounds about right.

I can take on look on my end once I’m back from lunch but off the top of my head I’m gonna guess that we might need to add webpack-cli as a dependency to our package.json file.

Maybe I had that globally installed?

…t to use locally installed version vs globally / temp version via npx
@sghoweri
Copy link
Contributor Author

sghoweri commented Aug 3, 2018

@bmuenzenmeyer - my latest push should fix that error you mentioned! Sorry about that -- totally missed that I hadn't wired that up to use a locally installed version of webpack-cli vs a globally installed version.

That part should now be good to go -- I'm now taking a quick peek at the merge conflicts on this.

# Conflicts:
#	packages/uikit-workshop/dist/index.html
#	packages/uikit-workshop/dist/styleguide/bower_components/EventEmitter.min.js
#	packages/uikit-workshop/dist/styleguide/bower_components/hogan-3.0.2.min.js
#	packages/uikit-workshop/dist/styleguide/bower_components/jquery.min.js
#	packages/uikit-workshop/dist/styleguide/bower_components/jwerty.min.js
#	packages/uikit-workshop/dist/styleguide/bower_components/prism.min.js
#	packages/uikit-workshop/dist/styleguide/bower_components/script.min.js
#	packages/uikit-workshop/dist/styleguide/bower_components/typeahead.bundle.min.js
#	packages/uikit-workshop/dist/styleguide/js/patternlab-pattern.js
#	packages/uikit-workshop/dist/styleguide/js/patternlab-pattern.min.js
#	packages/uikit-workshop/dist/styleguide/js/patternlab-viewer.js
#	packages/uikit-workshop/dist/styleguide/js/patternlab-viewer.min.js
#	packages/uikit-workshop/gulpfile.js
#	packages/uikit-workshop/package.json
#	packages/uikit-workshop/src/html/index.html
#	packages/uikit-workshop/src/scripts/components/modal-styleguide.js
@sghoweri
Copy link
Contributor Author

sghoweri commented Aug 3, 2018

@bmuenzenmeyer OK - I've gone in and resolved the couple of merge conflicts to get this up to date with the latest changes on dev, plus adjusted a couple small things in the JS based on some additional testing I've done on this.

There's a few CSS-related UI issues I'm aware of that I'll try to tackle this weekend (not yet sure if they're related to this PR itself or if they just existing issues -- if they're new it's almost certainly just a CleanCSS config to turn things down a little) but I'm pretty good about the JavaScript changes here all in all + how much better we'll be off once we start tackling some even more ambitious UI work 😉

@bmuenzenmeyer
Copy link
Member

@sghoweri this is building now and looks good on a quick glance on the 🚌
going to give it a closer look this tonight / weekend though.

nice work man! 💃 🕺 🌮 🍺 🎺 🥁 🎸

@bmuenzenmeyer
Copy link
Member

@sghoweri I've been travelling so this is taking a bit longer. Appreciate the patience!

@sghoweri
Copy link
Contributor Author

sghoweri commented Aug 8, 2018

@bmuenzenmeyer no worries - appreciate the update!

Copy link
Member

@bmuenzenmeyer bmuenzenmeyer left a comment

Choose a reason for hiding this comment

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

Amazing body of work Salem. Thanks for this.

@@ -18,6 +17,11 @@
"name": "Brian Muenzenmeyer",
"homepage": "http://brianmuenzenmeyer.com",
"role": "Developer"
},
{
"name": "Salem Ghoweri",
Copy link
Member

Choose a reason for hiding this comment

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

Well deserved!

@@ -46,11 +46,12 @@
</script>

<script id="pl-js-insert-{{ cacheBuster }}">
/*! loadJS: load a JS file asynchronously. [c]2014 @scottjehl, Filament Group, Inc. (Based on http://goo.gl/REQGQ by Paul Irish). Licensed MIT */
Copy link
Member

Choose a reason for hiding this comment

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

❤️

@bmuenzenmeyer bmuenzenmeyer merged commit 4adce73 into pattern-lab:dev Aug 14, 2018
antonia-rose pushed a commit to quelltexterin/nemo-uikit-workshop that referenced this pull request Apr 12, 2023
…-refactor-js

UIKit Refactor: Bower to NPM Migration, Partial JS Refactor, and New JS Build Pipeline
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.

2 participants