Skip to content

provide explicit global variable names to rollup#693

Merged
mwcz merged 5 commits intomasterfrom
fix-missing-global-variable-warning
Jan 22, 2020
Merged

provide explicit global variable names to rollup#693
mwcz merged 5 commits intomasterfrom
fix-missing-global-variable-warning

Conversation

@mwcz
Copy link
Copy Markdown
Contributor

@mwcz mwcz commented Jan 21, 2020

This PR removes the following warning when building elements.

(!) Missing global variable name
Use output.globals to specify browser global variable names corresponding to external modules
/home/mclayton/projects/pfe/elements/pfelement/dist/pfelement.umd (guessing 'PFElement')

It does so by using pfelement.className from each element's package.json to determine what the global variable name should be.

Testing instructions

git checkout fix-missing-global-variable-warning
npm run build

In the build log, you should not see any (!) Missing global variable name warnings. You can verify this more easily with grep.

git checkout fix-missing-global-variable-warning
npm run build 2>&1 | grep "Missing global"

That command should run for a while, but should not have any output.

For reference, when that command is run on master the output looks like the following:

$ npm run build 2>&1 | grep "Missing global"
@patternfly/pfe-icon-panel: (!) Missing global variable name
@patternfly/pfe-badge: (!) Missing global variable name
@patternfly/pfe-select: (!) Missing global variable name
@patternfly/pfe-avatar: (!) Missing global variable name
@patternfly/pfe-health-index: (!) Missing global variable name
@patternfly/pfe-datetime: (!) Missing global variable name
@patternfly/pfe-progress-indicator: (!) Missing global variable name
@patternfly/pfe-content-set: (!) Missing global variable names
@patternfly/pfe-page-status: (!) Missing global variable name
@patternfly/pfe-collapse: (!) Missing global variable name
@patternfly/pfe-icon-panel: (!) Missing global variable name
@patternfly/pfe-icon: (!) Missing global variable name
@patternfly/pfe-autocomplete: (!) Missing global variable name
@patternfly/pfe-cta: (!) Missing global variable name
@patternfly/pfe-card: (!) Missing global variable name
@patternfly/pfe-toast: (!) Missing global variable name
@patternfly/pfe-modal: (!) Missing global variable name
@patternfly/pfe-band: (!) Missing global variable name
@patternfly/pfe-badge: (!) Missing global variable name
@patternfly/pfe-markdown: (!) Missing global variable name
@patternfly/pfe-health-index: (!) Missing global variable name
@patternfly/pfe-progress-indicator: (!) Missing global variable name
@patternfly/pfe-datetime: (!) Missing global variable name
@patternfly/pfe-select: (!) Missing global variable name
@patternfly/pfe-page-status: (!) Missing global variable name
@patternfly/pfe-tabs: (!) Missing global variable name
@patternfly/pfe-content-set: (!) Missing global variable names
@patternfly/pfe-accordion: (!) Missing global variable name
@patternfly/pfe-number: (!) Missing global variable name
@patternfly/pfe-cta: (!) Missing global variable name
@patternfly/pfe-toast: (!) Missing global variable name
@patternfly/pfe-card: (!) Missing global variable name
@patternfly/pfe-modal: (!) Missing global variable name
@patternfly/pfe-collapse: (!) Missing global variable name
@patternfly/pfe-avatar: (!) Missing global variable name
@patternfly/pfe-icon: (!) Missing global variable name
@patternfly/pfe-band: (!) Missing global variable name
@patternfly/pfe-autocomplete: (!) Missing global variable name
@patternfly/pfe-navigation: (!) Missing global variable names
@patternfly/pfe-tabs: (!) Missing global variable name
@patternfly/pfe-accordion: (!) Missing global variable name
@patternfly/pfe-number: (!) Missing global variable name
@patternfly/pfe-markdown: (!) Missing global variable name
@patternfly/pfe-navigation: (!) Missing global variable names

@castastrophe
Copy link
Copy Markdown
Contributor

@mwcz I think I can guess how to test this but would you mind adding testing steps in the description to make it easier to replicate?

@castastrophe
Copy link
Copy Markdown
Contributor

Testing notes: Running npm install, the components build and we no longer see the missing global variable messaging, however, if any leftover folders exist from deleted components or from branch-switching, the build for every component fails with the following type of messaging:

@patternfly/pfe-accordion: [!] Error: Cannot find module '../../elements/pfe-layouts/package.json'
@patternfly/pfe-accordion: Error: Cannot find module '../../elements/pfe-layouts/package.json'
@patternfly/pfe-accordion:     at Function.Module._resolveFilename (internal/modules/cjs/loader.js:636:15)
@patternfly/pfe-accordion:     at Function.Module._load (internal/modules/cjs/loader.js:562:25)
@patternfly/pfe-accordion:     at Module.require (internal/modules/cjs/loader.js:692:17)
@patternfly/pfe-accordion:     at require (internal/modules/cjs/helpers.js:25:18)
@patternfly/pfe-accordion:     at fs.readdirSync.filter.map.dirent (/Users/carobert/Projects/patternfly-elements/elements/pfe-accordion/rollup.config.js:19:18)
@patternfly/pfe-accordion:     at Array.map (<anonymous>)
@patternfly/pfe-accordion:     at Object.<anonymous> (/Users/carobert/Projects/patternfly-elements/elements/pfe-accordion/rollup.config.js:19:4)
@patternfly/pfe-accordion:     at Module._compile (internal/modules/cjs/loader.js:778:30)
@patternfly/pfe-accordion:     at Object.require.extensions..js (/Users/carobert/Projects/patternfly-elements/node_modules/rollup/bin/rollup:1270:24)
@patternfly/pfe-accordion:     at Module.load (internal/modules/cjs/loader.js:653:32)

@mwcz
Copy link
Copy Markdown
Contributor Author

mwcz commented Jan 21, 2020

@castastrophe good to know, I'll look into reproducing that and coming up with a fix. It should be able to tolerate leftovers.

@mwcz
Copy link
Copy Markdown
Contributor Author

mwcz commented Jan 21, 2020

@castastrophe found and fixed, thanks!

@eyevana
Copy link
Copy Markdown
Contributor

eyevana commented Jan 21, 2020

Yay! I was running into the same issue. I just pulled the latest though, ran the build command again, and it looks great.

Random question...what does 2>&1 mean in the command you provided? @mwcz

Copy link
Copy Markdown
Contributor

@eyevana eyevana left a comment

Choose a reason for hiding this comment

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

lyric galleria thingamabob massive

@mwcz
Copy link
Copy Markdown
Contributor Author

mwcz commented Jan 21, 2020

@eyevana Great question! POSIX shells like bash have two types of output, stdout (for regular messages) and stderr (for errors). Both types of output appear to you in the terminal, but when you pipe a command into another command, as I'm piping into grep, only stdout is piped. stderr is still printed to the screen.

2>&1 means "redirect stderr into stdout", so it becomes a single stream of text, and all of it gets passed into grep. Basically, I added it so I could say "this command should output no text". Without the 2>&1, the build command would have piped stdout into grep, but stderr would still print to screen looking something like this:

WARN deprecated --include-filtered-dependencies has been renamed --include-dependencies
lerna notice cli v3.20.2
lerna notice filter including "*/pfe-avatar"
lerna notice filter including dependencies
lerna info filter [ '*/pfe-avatar' ]
lerna info Executing command in 3 packages: "npm run build"
@patternfly/pfe-avatar: ./_temp/pfe-avatar.js → ./dist/pfe-avatar.js...
@patternfly/pfelement: ./_temp/pfelement.js → ./dist/pfelement.js...
@patternfly/pfe-avatar: created ./dist/pfe-avatar.js in 72ms
@patternfly/pfe-avatar: ./_temp/pfe-avatar.umd.js → ./dist/pfe-avatar.umd.js...
@patternfly/pfelement: created ./dist/pfelement.js in 72ms
@patternfly/pfelement: ./_temp/pfelement.umd.js → ./dist/pfelement.umd.js...
@patternfly/pfe-avatar: created ./dist/pfe-avatar.umd.js in 551ms
@patternfly/pfe-avatar: ./_temp/pfe-avatar.js → ./dist/pfe-avatar.min.js...
@patternfly/pfelement: created ./dist/pfelement.umd.js in 556ms
@patternfly/pfelement: ./_temp/pfelement.js → ./dist/pfelement.min.js...
@patternfly/pfelement: created ./dist/pfelement.min.js in 474ms
@patternfly/pfelement: ./_temp/pfelement.umd.js → ./dist/pfelement.umd.min.js...
@patternfly/pfe-avatar: created ./dist/pfe-avatar.min.js in 566ms
@patternfly/pfe-avatar: ./_temp/pfe-avatar.umd.js → ./dist/pfe-avatar.umd.min.js...
@patternfly/pfe-avatar: created ./dist/pfe-avatar.umd.min.js in 585ms
@patternfly/pfelement: created ./dist/pfelement.umd.min.js in 621ms
lerna success run Ran npm script 'build' in 3 packages in 3.8s:
lerna success - @patternfly/pfe-avatar
lerna success - @patternfly/pfe-sass
lerna success - @patternfly/pfelement

(Note that none of these messages are actually errors... I think some part of our build system is logging things as errors that aren't actually errors. 🤔 )

More about standard streams

@mwcz mwcz merged commit 21fe2d7 into master Jan 22, 2020
@castastrophe castastrophe deleted the fix-missing-global-variable-warning branch January 27, 2020 14:54
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.

3 participants