fix resolve bindings issue#1919
Conversation
🦋 Changeset detectedLatest commit: 0ea1044 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
✅ Deploy Preview for compiled-css-in-js canceled.
|
| gridTemplateColumns: '1fr 1fr', | ||
| flexWrap: 'wrap', | ||
| flexDirection: 'column', | ||
| [akMedia.above.sm]: { |
There was a problem hiding this comment.
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.
| try { | ||
| modulePath = resolveRequest(moduleImportSource, extensions, meta); | ||
| } catch { | ||
| // Treat unresolved userland imports as dynamic values when they are unavailable | ||
| // from the Compiled plugin's module resolution context. | ||
| return; | ||
| } |
There was a problem hiding this comment.
I will say, this is one of those things I liked throwing errors before, but it didn't do this consistently whatsoever…
| `, | ||
| { filename: 'checkbox-list.jsx' } | ||
| ); | ||
| }).not.toThrow(); |
There was a problem hiding this comment.
Instead of assert not toThrow, can you have a snapshot assertion? It will be good to see what this gets transformed to.
| let modulePath: string; | ||
|
|
||
| try { | ||
| modulePath = resolveRequest(moduleImportSource, extensions, meta); |
There was a problem hiding this comment.
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',
}
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@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.
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.
There was a problem hiding this comment.
@kylorhall-atlassian @liamqma , found there are more cases throwing errors, like this one:

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.
There was a problem hiding this comment.
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,
)}
There was a problem hiding this comment.
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,
)}
/>
| return resolve.sync(id, { | ||
| return resolve.sync(request, { | ||
| // Resolve imports from the file being transformed, not from this plugin's package. | ||
| basedir: dirname(filename), |
There was a problem hiding this comment.
from reading the description of a PR, would just this line be sufficient? why is packageFilter and pathFilter needed?
There was a problem hiding this comment.
tried in my spike, but didn't work unfortunately with basedir only for feature flag cases.
FeatureGates usage is like
import FeatureGates from '@atlaskit/feature-gate-js-client/feature-gates';
<Box xcss={cx(
marginTop: 16,
FeatureGates.getExperimentValue('xxx', 'isEnabled', false) && {
marginTop: 0,
})
/>
There was a problem hiding this comment.
Wait, I think AFM has some atlassian-resolver that handles changes like this. The code path if (!resolver) { ... } should NOT be triggered at AFM. An error like this means the Compiled resolver is not configured properly for the situation the build runs against. It might be easier to switch the resolver at AFM rather than applying the fix here.
There was a problem hiding this comment.
@liamqma I would say as well, given we have the resolver config in probably 100+ places at Atlassian, it very well might be worth insourcing it—I think Atlaspack/Compiled could possibly even make it better than asking every consumer to build and maintain their own resolver logic. I do agree though, just when it's a problem in a repo that already has the resolvers configured for the past ~4 years it might need to be centralized.
There was a problem hiding this comment.
@kylorhall-atlassian @liamqma I insourced @atlaskit/resolvers/compiled-resolver as a default resolver if no resolver is found from babel plugin option.
I will verify in my AFM spike
| import { media as subpathMedia } from "@compiled-private/source-relative-responsive/media"; | ||
| export const CheckboxList = ({ testId = "checkbox-list" }) => ( | ||
| <AkBox | ||
| xcss={akXcss({ |
There was a problem hiding this comment.
I am a bit confused. After the transformation, should this snapshot look like
<AkBox className="..." />
?
There was a problem hiding this comment.
other existing test cases are the same, I think it's because the test util it used which made it run in runtime mode.
| '@compiled/babel-plugin': patch | ||
| --- | ||
|
|
||
| Resolve module imports relative to the transformed file when resolving bindings. |
There was a problem hiding this comment.
Is this issue specific to compiled plugin? Or does it need to be ported over atlaspack SWC plugins?

What is this change?
This updates
@compiled/babel-pluginbinding resolution so imported bindings are resolved from the file being transformed rather than from the Compiled plugin package.It also adds support for simple package
exportsresolution when using theresolvepackage:import '@pkg'→exports['.']import '@pkg/foo'→exports['./foo']A regression test was added for
xcssusage that references values imported from another package via both root and subpath exports.Verified in AFM Spike PR with yarn patch: https://bitbucket.org/atlassian/atlassian-frontend-monorepo/pull-requests/413960 (build is green)
Why are we making this change?
Compiled performs binding resolution while statically evaluating styles used in paths like
xcss/cssMap.Some valid source code includes imported values inside style expressions or
xcssexpressions, for example:Before this change, package imports encountered during binding resolution were resolved without a basedir. That meant the resolve package tried to resolve them from the Compiled plugin package location, for example:
node_modules/@compiled/babel-plugin/dist/utils
instead of from the source file being transformed.
This caused false MODULE_NOT_FOUND errors for otherwise valid imports, such as:
Cannot find module '@atlaskit/feature-gate-js-client'
from '.../node_modules/@compiled/babel-plugin/dist/utils'
or:
Cannot find module '@atlaskit/feature-gate-js-client/feature-gates'
from '.../node_modules/@compiled/babel-plugin/dist/utils'
This is broader than responsive media usage. It can happen for feature gates, constants, helpers, or any imported package value that Compiled encounters during binding analysis.
How are we making this change?
(Optional.)
PR checklist
Don't delete me!
I have...