Skip to content

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

Merged
lynamemi merged 5 commits intomicrosoft:masterfrom
lynamemi:fix-sideeffects
Jun 7, 2018
Merged

Website: Fix sideEffects for website tree shaking and add notes about testing#5133
lynamemi merged 5 commits intomicrosoft:masterfrom
lynamemi:fix-sideeffects

Conversation

@lynamemi
Copy link
Copy Markdown
Collaborator

@lynamemi lynamemi commented Jun 7, 2018

Pull request checklist

[ ] Addresses an existing issue: Fixes #0000
[X] Include a change request file using $ npm run change

Description of changes

In #5126 , I attempted to enable sideEffects for stylesheets in ouifr using global styles. This PR fixes the syntax so it works.

For the files in the array, I tried .js, .ts, .d.ts, .scss., and .css file types and none worked. That's why I'm going with the wildcard (*) file type.

I also added documentation for the website about how to test minified files locally so we can better test changes like this in the future (I did test this change using the method outlined).

Focus areas to test

(optional)

Microsoft Reviewers: Open in CodeFlow

var scripts = [
'//cdnjs.cloudflare.com/ajax/libs/react/16.3.1/umd/react.production.min.js',
'//cdnjs.cloudflare.com/ajax/libs/react-dom/16.3.1/umd/react-dom.production.min.js',
// To serve minified files locally for testing purposes, change the next line to './dist/fabric-sitev5.min.js'
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.

This works for sure, but is pretty hacky. We could consider adding something like this to make it a little easier:

var isLocal = window.location.hostname === 'localhost';
var appPath = isLocal ? host + 'dist/' : '';

var scripts = [
  '//cdnjs.cloudflare.com/ajax/libs/react/0.14.7/react.js',
  '//cdnjs.cloudflare.com/ajax/libs/react/0.14.7/react-dom.js',
  appPath + 'fabric-sitev5.min.js'
];


## Test serving minified files locally
1. `npm install -g http-server`
2. In index.html, change `'/fabric-sitev5.js'` to `'./dist/fabric-sitev5.min.js'` (see comment in code).
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.

See comment below--we can make this bit simpler

@lynamemi lynamemi closed this Jun 7, 2018
@lynamemi lynamemi reopened this Jun 7, 2018
@lynamemi lynamemi merged commit 6a22771 into microsoft:master Jun 7, 2018
@lynamemi lynamemi deleted the fix-sideeffects branch June 7, 2018 23:29
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.

2 participants