Skip to content

Adds webpack-utils package; a fabric-optimized async loader #5122

Merged
kenotron merged 13 commits intomicrosoft:masterfrom
kenotron:loader
Jun 7, 2018
Merged

Adds webpack-utils package; a fabric-optimized async loader #5122
kenotron merged 13 commits intomicrosoft:masterfrom
kenotron:loader

Conversation

@kenotron
Copy link
Copy Markdown
Contributor

@kenotron kenotron commented Jun 6, 2018

Pull request checklist

  • Addresses an existing issue: Fixes #0000
  • Include a change request file using $ npm run change
  • Microsoft Alias (if you have one):

Description of changes

With this change, I will add a new package dedicated to anything that we would do to optimize webpack usage with fabric. The first of these utils is the fabricAsyncLoader - it will allow apps to save on bundle size by move non-boot controls to be loaded asynchronously (e.g. ContextualMenu inside Button).

Focus areas to test

(optional)

Microsoft Reviewers: Open in CodeFlow

@kenotron kenotron requested a review from dzearing June 6, 2018 20:59
"extends": ["office-ui-fabric-react-tslint"],
"rules": {
"deprecation": false,
"prefer-const": false
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Why is this turned off? :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Just copied from utilities :P

@@ -0,0 +1,7 @@
let path = require('path');
let { createConfig } = require('../../scripts/tasks/jest-resources');
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Both of these can be consts?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes - again, copied from utilities :)

`yarn add -D @uifabric/webpack-utils`


## Fabric Async Loader
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Super cool! :D

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

👍

@@ -0,0 +1,2 @@
registry=https://registry.npmjs.org/
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Isn't this the default? If so, do we need this?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

oh wow, I keep copying stuff that I don't really need!

Copy link
Copy Markdown
Contributor

@cliffkoh cliffkoh left a comment

Choose a reason for hiding this comment

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

Need more time to review..

@@ -0,0 +1,6 @@
{
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

API extractor isn't currently running and besides the entries in here are wrong.. :)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The build script is geared towards building lib and lib-commonjs which isn't desired in webpack utils

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

So I will delete this file then.

{
"packageName": "@uifabric/webpack-utils",
"comment": "Creates a new webpack-utils package",
"type": "major"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Your first version published is going to be 2.0.0...

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

yeah maybe update the version to a 0, and do minor bumps until you're ready to do 1.0

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

ok

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I cranked it down to 0.0.1 with a minor change file

].join('');
};

export = fabricAsyncLoader;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please add documentation to this file and also just use es6 syntax please :) (be consistent with rest of repo)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I can't use es6 syntax. Loaders need to be exported this way. I can add a little bit of doc but this loader doesn't do much yet (if ever?)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

doc added

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I had thought though that the .ts file is not used as the loader immediately, and in the compilation to js, this will turn into commonjs syntax and thereby become compatible...?

@kenotron kenotron merged commit 08cd29e into microsoft:master Jun 7, 2018
@kenotron kenotron deleted the loader branch June 7, 2018 22:05
oengusmacinog-zz added a commit that referenced this pull request Jun 11, 2018
* TeachingBubble/Coachmark:  Fix TeachingBubble content wrapping to next line unnecessarily  (#5121)

* Fix teaching  bubble breaking text into next line.

* Add change file

* Fix isWide variation of teaching bubble not inheriting max-width.

* Customizable styles update (#5134)

* @Customizable add styles to args

* change log

* Adds webpack-utils package; a fabric-optimized async loader  (#5122)

* adding a new fabric webpack plugin

* adds fabric webpack-utils package

* adds a peer dep definition

* revert office-ui-fabric-react changes and adds a change file

* addressing PR comments

* moving down versions

* api-extractor isn't needed

* added some documentation for the loader

* make sure not to publish for now, will publish after an initial manual push

* adding some non-consequential prettier changes

* Updated README.md so that the instruction is correct

* Update npm version to 5.4.0 (#5137)

* Website: Fix sideEffects for website tree shaking and add notes about testing (#5133)

* Fix sideEffects

* Update readme and add comments for testing minified files process

* Added change files

* Adding built in  local check for index and removing webpack.config bits.

* Fix example app base (#5139)

* Fixing example app base.

* updating change file

* ContextualMenu: fix hover click submenu behavior (#5141)

* ContextualMenu: Remove the ability for click to close a submenu. This aligns with windows behavior

* rush change

* Dropdown accessibility (#5138)

* add listener in Callout for blur

* Revert "add listener in Callout for blur"

This reverts commit bb9c01a.

* add option to active descendant and aria-label to option

* change request file

* update Dropdown snapshot tests after adding aria-label

* Shimmer: adding new component to OUFR (#5067)

* Brings an initial full copy of Shimmer from experiments.

* Fix all imports.
Remove ShimmerTile.

* Rename IStyleFunction to IStyleFunctonOrObject

* Renames getStyles to styles

* Removes all deprecated props and warnings inherited from experiments package.

* Adds markdown documentation.
Adds 1 snapshot test.

* Updates snapshot tests.

* Adds a functional test to Shimmer

* Write a test story-book

* Story-book.

* Adds more stories to the screener tests.

* Adds one more story for custom elements.

* Adds Shimmer to Fabric-website

* Rush change logs.

* Fixes a visual bug noticed in Fabric website.

* Upsss... updates snapshots.

* Formats everything according to new prettier rules.

* More fixes for prettier formating rules.

* Removes enum for verticalAlign prop and replaces it with strings.

* Refactor the width prop to adress feedback.

* Updates the snapshots.

* Removes global styles for examples.

* Fixes screener story for Shimmer.

* Adds native props to the outer container.

* Updates snapshots.

* Fixes CI release job that passes --production flag to webpack-utils' tsc call (#5147)

* adding a build script for webpack-utils because it builds differently than all other packages

* adding change file

* Revert back to npm 5.3.0

* basic setup for Fluent Styles page
@microsoft microsoft locked as resolved and limited conversation to collaborators Aug 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants