Skip to content

fix resolve bindings issue #1919

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.

Sign up for GitHub

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

Open
xtan-atlas wants to merge 6 commits into
base: master
Choose a base branch
from
+221 −10
Open

fix resolve bindings issue #1919

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c4da5fc
fix resolve bindings issue
xtan-atlas Jul 14, 2026
00e55cc
fix resolve binding issue
xtan-atlas Jul 14, 2026
aab240c
improve resolveRequest
xtan-atlas Jul 14, 2026
a135598
insource @atlaskit/resolvers/compiled-resolver
xtan-atlas Jul 16, 2026
9f4ecd3
fix type error
xtan-atlas Jul 16, 2026
0ea1044
fix test issues
xtan-atlas Jul 16, 2026
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
5 changes: 5 additions & 0 deletions .changeset/quiet-imports-resolve.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@compiled/babel-plugin': patch
---

Treat unresolved module imports as dynamic values when resolving bindings.
10 changes: 9 additions & 1 deletion packages/babel-plugin/src/utils/resolve-binding.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,15 @@ export const resolveBinding = (
}

const extensions = meta.state.opts.extensions ?? DEFAULT_CODE_EXTENSIONS;
const modulePath = resolveRequest(moduleImportSource, extensions, meta);
let modulePath: string;

try {
modulePath = resolveRequest(moduleImportSource, extensions, meta);

@liamqma liamqma Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I have some concerns about this approach.

When a "cannot find module X" error occurs, it most likely means the compiled resolver is not configured properly. We should fix the resolver configuration instead.

I understand that configuring a resolver to cover many use cases can be difficult. We actually came across a similar issue with media.something.something before. Could we resolve it by inlining the value? For example:

@media (min-width: 600px) {
    flexWrap: 'nowrap',
}

@kylorhall-atlassian kylorhall-atlassian Jul 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

My understanding is media.above.md has always worked in css(), but always through an error in cssMap(); I'm unclear what the difference is if this results in a compile-checked 1:1 output. Is this just a false error?

@kylorhall-atlassian kylorhall-atlassian Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I am fine with us codemodding away media.x.y 1:1 as well, literally deprecate, ratchet, and remove in AFM if it's a blocker and I'll redo the docs.

@liamqma liamqma Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks Kylor!
@xtan-atlas would you be able to replace media.x.y references with their corresponding inline values on a 1:1 basis, if there are not too many occurrences? I think this aligns well with our long term strategy: all variables should be localized, with no cross-file resolution at build time.

If you consider inlining the media.x.y values unsafe, I completely understand. Merging this PR as is works for me too.

@xtan-atlas xtan-atlas Jul 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@kylorhall-atlassian @liamqma I actually got it working and verified in AFM without patch by avoid inline xcss. So I can bump deps without this blocking me.

https://bitbucket.org/atlassian/atlassian-frontend-monorepo/pull-requests/413472/diff#chg-adminhub/packages/common/checkbox-list/src/ui/checkbox-list.tsx

I don't think inline is a good, we officially support media.xxx in https://atlassian.design/components/primitives/responsive/usage

But we may just need an eslint rule to cover.

@xtan-atlas xtan-atlas Jul 14, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@kylorhall-atlassian @liamqma , found there are more cases throwing errors, like this one:
Screenshot 2026-07-14 at 5 51 34 pm

Basically, if the inlined xcss have used variables imported, it'd break. I can't get the AFM PR green without patches or more bump from @compiled/babel-plugin... So I think it'd be better to fix properly in Compiled first.

I've updated the resolveRequest function, and tested in AFM Spike: https://bitbucket.org/atlassian/atlassian-frontend-monorepo/pull-requests/413960

I added a unit test with snapshot inline as suggested, please take a look.

@liamqma liamqma Jul 15, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

OH good catch!
I see what's broken

xcss={cx(
	styles.readViewButton,
	FeatureGates.getExperimentValue('jira-aifc-ux-papercuts-sprt', 'isEnabled', false, {
		fireExperimentExposure: false,
	}) && styles.hoverStyles,
)}

@xtan-atlas xtan-atlas Jul 15, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

yes this.

Another example below, which need pathFilter in resolveRequest to fix:

import { ConversationAssistantUIModes } from '@atlassian/conversation-assistant/types';

<Box
	xcss={cx(
		styles.animatedMiniModalBefore,
		mode === ConversationAssistantUIModes.MINI_MODAL && styles.animatedMiniModalAfter,
	)}
/>

} catch {
// Treat unresolved userland imports as dynamic values when they are unavailable
// from the Compiled plugin's module resolution context.
return;
}

@kylorhall-atlassian kylorhall-atlassian Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I will say, this is one of those things I liked throwing errors before, but it didn't do this consistently whatsoever…


if (!extensions.some((extension) => modulePath.endsWith(extension))) {
// Don't attempt to parse any files that are not configured as code
Expand Down
27 changes: 27 additions & 0 deletions packages/babel-plugin/src/xcss-prop/__tests__/transformation.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,33 @@ describe('xcss prop transformation', () => {
`);
});

it('should not throw when primitive xcss uses unresolved responsive imports', () => {
expect(() => {
transform(
`
import { Box as AkBox, xcss as akXcss } from '@atlaskit/primitives';
import { media as akMedia } from '@atlaskit/primitives/responsive';

export const CheckboxList = ({ testId = 'checkbox-list' }) => (
<AkBox
xcss={akXcss({
display: 'grid',
gridTemplateColumns: '1fr 1fr',
flexWrap: 'wrap',
flexDirection: 'column',
[akMedia.above.sm]: {

@kylorhall-atlassian kylorhall-atlassian Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Oh, I see, it's about this…yeah, media needs to be fixed/removed/something'd.

@atlaskit/css with bounded interface is the way to solve it, but we do not have a solution as Compiled decided to drop the work to make global cached tokens/constants a thing.

flexWrap: 'nowrap',
},
})}
testId={testId}
/>
);
`,
{ filename: 'checkbox-list.jsx' }
);
}).not.toThrow();

@liamqma liamqma Jul 14, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Instead of assert not toThrow, can you have a snapshot assertion? It will be good to see what this gets transformed to.

});

it('should allow ternaries', () => {
const result = transform(`
import { cssMap } from '@compiled/react';
Expand Down
Loading