-
Notifications
You must be signed in to change notification settings - Fork 23
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
build: Introduce rollup to bundle percy-agent.js
for browsers
#124
Conversation
This is to resolve the error where winston was being being included in various webpack bundles. We change the `main` entry point of the `@percy/agent` package so only the file that's ready for a browser is bundled. Then we transpile that file fully with babel and rollup so their webpack config isn't looking to compile it too. This solves percy/percy-cypress#58 fully.
Most of the errors stemmed from us using the logger (which is a node logger) in our SDK utils, which is apart of our main export of the package. This would cause bundlers to try and bundle that code and fail because it couldn't find FS
That will be handled in a different PR and we're changing the way SDKs import that file
20ca9f8
to
ecdb395
Compare
percy-agent.js
& remove loggerpercy-agent.js
for browsers
"@types/sinon-chai": "^3.2.0", | ||
"@semantic-release/git": "^7.0.8", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks
@@ -109,7 +118,7 @@ | |||
"repository": "percy/percy-agent", | |||
"scripts": { | |||
"build": "npm run clean && rm -rf dist && tsc && npm run build-client", | |||
"build-client": "mkdir -p dist/public && browserify src/percy-agent-client/index.ts -p [ tsify --noImplicitAny ] --standalone PercyAgent > dist/public/percy-agent.js", | |||
"build-client": "rollup -c", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🍍 LGTM. Awesome work on this @Robdel12.
Next up we'll refactor npm run build-client-test
to avoid using browserify and then we can remove browserify completely.
Really curious what |
What is this?
This PR introduces the rollup bundler so we can properly bundle the
percy-agent.js
file. Currently we transpile it with browserify but we don't bundle all of the code together into a single file. Since we're injecting this code into browsers it would be best to transpile it and then pull it together into a single file (so there are norequires
or anything of that nature in the file).This also introduces babel, which will soon be replacing all of browserify in this project. The community has essentially settled on babel as the ecosystem to transpile future JS code to JS that can run in the browser today.