De-duplicate ifTrueBundles and ifFalseBundles arrays when creating conditional manifest

[dddlr]

Conditional bundling bug fix:

Add de-duplication so that assets don't appear duplicated inside the conditional manifest.

In other words, we want to de-duplicate stuff like this:

Motivation

When importCond is used twice in the same bundle with the exact same arguments, we will add the conditional bundles to ifTrueBundles and ifFalseBundles twice.

For example, imagine we have two files that get bundled into the same asset (let's call it navigation-stuff.4abe98ff.js) at build-time:

// src/navigation/dev-panel/index.tsx

import React from 'react';

const Component = importCond('my-feature-gate', '@private/component/index-new.tsx', '@private/component/index-old.tsx')
export const DevPanel = (props) => {
    return (
        <>
            <DevStuff />
            <Component ... />
            {/* and so on... */}
        </>
    );
};

// src/navigation/product-panel/index.tsx

import React from 'react';

const Component = importCond('my-feature-gate', '@private/component/index-new.tsx', '@private/component/index-old.tsx')
export const ProductPanel = (props) => {
    return (
        <>
            <ProductStuff ... />
            <Component ... />
            {/* and so on... */}
        </>
    );
};

index-new.js and index-old.js will get added to the ifTrueBundles and ifFalseBundles arrays for navigation-stuff.4abe98ff.js twice, because there are two importCond function calls that end up in the same bundle.

Changes

Add some basic de-duplication in packages/reporters/conditional-manifest/src/ConditionalManifestReporter.js, which is where we create the conditional manifest.

I considered adding this de-duplication into getConditionalBundleMapping directly too, but it seemed superfluous - @JakeLane let me know if you have a different opinion here.

Note that this PR will need to be updated once #640 is merged in

Checklist

• Make a PR to add de-duplication to the old Jira SSR too
• Existing or new tests cover this change
• There is a changeset for this change, or one is not required

[changeset-bot]

🦋 Changeset detected

Latest commit: d624be3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 102 packages

Name	Type
@atlaspack/reporter-conditional-manifest	Patch
@atlaspack/integration-tests	Patch
@atlaspack/feature-flags	Patch
@atlaspack/bundler-default	Patch
@atlaspack/utils	Patch
@atlaspack/bundler-experimental	Patch
@atlaspack/cache	Patch
@atlaspack/cli	Patch
@atlaspack/core	Patch
@atlaspack/fs	Patch
@atlaspack/graph	Patch
@atlaspack/types-internal	Patch
@atlaspack/optimizer-inline-requires	Patch
@atlaspack/packager-html	Patch
@atlaspack/packager-js	Patch
@atlaspack/reporter-cli	Patch
@atlaspack/runtime-js	Patch
@atlaspack/transformer-js	Patch
@atlaspack/node-resolver-core	Patch
@atlaspack/config-default	Patch
@atlaspack/config-repl	Patch
@atlaspack/package-manager	Patch
@atlaspack/test-utils	Patch
@atlaspack/workers	Patch
@atlaspack/link	Patch
@atlaspack/bundle-stats	Patch
@atlaspack/optimizer-blob-url	Patch
@atlaspack/optimizer-css	Patch
@atlaspack/optimizer-data-url	Patch
@atlaspack/optimizer-image	Patch
@atlaspack/optimizer-svgo	Patch
@atlaspack/optimizer-swc	Patch
@atlaspack/optimizer-terser	Patch
@atlaspack/packager-css	Patch
@atlaspack/packager-raw-url	Patch
@atlaspack/packager-svg	Patch
@atlaspack/packager-webextension	Patch
@atlaspack/packager-xml	Patch
@atlaspack/reporter-build-metrics	Patch
@atlaspack/reporter-bundle-analyzer	Patch
@atlaspack/reporter-bundle-stats	Patch
@atlaspack/reporter-dev-server-sw	Patch
@atlaspack/reporter-dev-server	Patch
@atlaspack/reporter-json	Patch
@atlaspack/reporter-lsp	Patch
@atlaspack/reporter-sourcemap-visualiser	Patch
@atlaspack/reporter-tracer	Patch
@atlaspack/resolver-glob	Patch
@atlaspack/runtime-browser-hmr	Patch
@atlaspack/runtime-react-refresh	Patch
@atlaspack/runtime-service-worker	Patch
@atlaspack/runtime-webextension	Patch
@atlaspack/transformer-babel	Patch
@atlaspack/transformer-css	Patch
@atlaspack/transformer-image	Patch
@atlaspack/transformer-postcss	Patch
@atlaspack/transformer-posthtml	Patch
@atlaspack/transformer-react-refresh-wrap	Patch
@atlaspack/transformer-typescript-types	Patch
@atlaspack/transformer-webextension	Patch
@atlaspack/transformer-webmanifest	Patch
@atlaspack/watcher-watchman-js	Patch
@atlaspack/validator-eslint	Patch
@atlaspack/validator-typescript	Patch
@atlaspack/query	Patch
@atlaspack/register	Patch
@atlaspack/transformer-html	Patch
@atlaspack/profiler	Patch
@atlaspack/types	Patch
@atlaspack/resolver-default	Patch
@atlaspack/config-webextension	Patch
@atlaspack/plugin	Patch
@atlaspack/transformer-jsonld	Patch
@atlaspack/bundler-library	Patch
@atlaspack/compressor-brotli	Patch
@atlaspack/compressor-gzip	Patch
@atlaspack/compressor-raw	Patch
@atlaspack/namer-default	Patch
@atlaspack/optimizer-cssnano	Patch
@atlaspack/optimizer-htmlnano	Patch
@atlaspack/packager-raw	Patch
@atlaspack/packager-ts	Patch
@atlaspack/packager-wasm	Patch
@atlaspack/reporter-bundle-buddy	Patch
@atlaspack/resolver-repl-runtimes	Patch
@atlaspack/transformer-glsl	Patch
@atlaspack/transformer-graphql	Patch
@atlaspack/transformer-inline-string	Patch
@atlaspack/transformer-inline	Patch
@atlaspack/transformer-json	Patch
@atlaspack/transformer-less	Patch
@atlaspack/transformer-mdx	Patch
@atlaspack/transformer-pug	Patch
@atlaspack/transformer-raw	Patch
@atlaspack/transformer-sass	Patch
@atlaspack/transformer-svg-react	Patch
@atlaspack/transformer-svg	Patch
@atlaspack/transformer-toml	Patch
@atlaspack/transformer-typescript-tsc	Patch
@atlaspack/transformer-worklet	Patch
@atlaspack/transformer-xml	Patch
@atlaspack/transformer-yaml	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[dddlr]

@atlaspack/bundler-default and @atlaspack/utils were included by default, is this correct?

[marcins]

Is it because of the change in feature flags? Though I'd expect more packages to be touched by that?

[JakeLane]

yeah this seems incorrect, the changeset file doesn't list dependents - that's handled in the business logic of the github action

[marcins]

Probably not important right now but might be a factor as the feature scales - but does it make sense to do the sort/set for every bundle, or do it once as a pass at the end?

Maybe even consider something like using two maps like ifTrueBundles: Map<string, Set<string>> = new Map() (and one for false), adding to the sets in this pass, and at the end producing the bundleInfo object and sorting the sets then.

[JakeLane]

@dddlr might be a good opportunity to improve this code, given you've raised a PR to mitigate in jira - you can take your time on this one

[JakeLane]

looks good! Did you check if the JSRuntime code needs to be updated?

[dddlr]

looks good! Did you check if the JSRuntime code needs to be updated?

yeah, i updated the conditional bundling example locally to try to reproduce the issue, as well as looking into the edition_awareness_upsell... usages in the jira build output, and they don't contain any duplicates as far as i can tell...?

not quite sure why though, will have to dig into the implementation (especially the getConditionsForDependencies function)