Skip to content
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

fix journey manager umd #127

Merged
merged 2 commits into from
Jul 12, 2024
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions packages/rollup-config-nlx/index.ts
Original file line number Diff line number Diff line change
@@ -65,9 +65,7 @@ export default function buildconfig({
commonjs(), // needed to bundle node modules that use cjs
json(), // needed to fetch package version from package.json
nodeResolve({ browser: true }), // bundles packages from node_modules
nodePolyfills({
include: "../../node_modules/**/*.js",
Copy link
Collaborator Author

@michaelglass michaelglass Jul 11, 2024

Choose a reason for hiding this comment

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

before #119 this value was "../**/node_modules/**/*.js" (presumably taken from ionic-team/rollup-plugin-node-polyfills#17 (comment)).

In 119 I changed it to ../.. to point to the proper node_modules path which may have caused this regression.

I still don't understand why ../**/node_modules is relevant. I believe in the comment linked above, it's for a different monorepo setup where node_modules are symlinked into package directories.

In any case, the change to ../../ broke things. I tried to avoid regressions like this by diffing umd output before and after #119 changes, but apparently that testing was insufficient!

Do you understand what the previous include param was doing? If not I'd rather drop it altogether (like I do in this PR)

}),
nodePolyfills({ include: null }),
michaelglass marked this conversation as resolved.
Show resolved Hide resolved
terser(),
replace({
preventAssignment: true,