Split packages/core into packages/core and packages/browser#1411
Split packages/core into packages/core and packages/browser#1411carterworks wants to merge 68 commits intomainfrom
Conversation
|
|
Let's merge this after the release. |
# Conflicts: # package.json # packages/browser/test/integration/helpers/mocks/emptyEventResponse.json # packages/browser/test/integration/helpers/mocks/mediaSessionResponse.json # packages/browser/test/integration/helpers/mocks/targetCustomCodeAction.json # packages/browser/test/integration/helpers/mocks/targetPrependHtmlAction.json # packages/browser/test/integration/specs/Advertising/consent_gate.spec.js # packages/core/package.json # packages/core/test/unit/specs/components/Advertising/handlers/viewThroughHandler.spec.js
# Conflicts: # package.json # packages/browser/test/integration/specs/Advertising/clickthrough_duplicate_skwcid.spec.js
| OF ANY KIND, either express or implied. See the License for the specific language | ||
| governing permissions and limitations under the License. | ||
| */ | ||
| import fs from "fs"; |
There was a problem hiding this comment.
I find it a little weird that in a file from src we are using Node APIs. If this file would be read by a bundler, it might crash. What do you think?
There was a problem hiding this comment.
It's actually the other way around - rollup polyfils calls to fs, so if it is called by the browser then it will crash.
It is looking more and more likely that I need to abandon the goal of being buildless.
There was a problem hiding this comment.
You and I talked in person and discovered that the functionality of componentMetadata.js can be moved into the alloyBuilder, because that is the only place that it is used.
- do that
| await makeSendServiceWorkerTrackingData( | ||
| { | ||
| /* eslint-disable-next-line no-underscore-dangle */ | ||
| xdm: data._xdm.mixins, |
There was a problem hiding this comment.
Will data always have an _xdm key?
There was a problem hiding this comment.
It's just a copy-paste from your serviceWorker.js file, so it will depend on what gets attached to notificationclose service worker events, as it did before.
See:
alloy/packages/core/src/serviceWorker.js
Line 77 in 366cc96
alloy/packages/browser/src/serviceWorker.js
Lines 79 to 80 in 02f6385
| * @param {NotificationEvent} event | ||
| */ | ||
| onNotificationClick(event) { | ||
| serviceWorkerNotificationClickListener({ |
There was a problem hiding this comment.
onPush returns a listener while onNotificationClick returns undefined. Should we return here too?
|
There is something wrong with the linting. Running |
| "build:clean": "rimraf dist distTest", | ||
| "build:types": "tsc -p jsconfig.json --declaration --emitDeclarationOnly --outDir dist/types --checkJs false --noEmit false", | ||
| "build": "pnpm run build:clean && rollup -c --environment BASE_CODE_MIN,STANDALONE,STANDALONE_MIN,SERVICE_WORKER,SERVICE_WORKER_MIN,BUNDLESIZE && pnpm run build:types", | ||
| "build:watch": "rollup -c --watch", |
|
How do you run the dev command now that was removed from the root package.json? |
|
In rollup.config.js there are some typos: brotili instead of brotli. |

Warning
This PR is based off of #1414, which itself is based off of #1412, so this should not be merged until they are.Description
This PR changes the publishing artifact from being the packages/core folder to a new folder called packages/browser. And that folder gets the @adobe/alloy, and then the core folder appropriately gets the name @adobe/alloy-core. The intent is that both of these packages will be published to NPM.
The core package will be buildless, and then all build steps as well as integration and functional tests, anything to do with a browser stays in the browser package.
No functionality or browser specific functionality has been removed from the packages, the core package, but that is kind of the next step, and hopefully that work can be parallelized.
As it stands right now, the CI test and the publishing workflow still need to be completed. But right now this branch is 95% done.Related Issue
Motivation and Context
Screenshots (if appropriate):
Checklist: