-
Notifications
You must be signed in to change notification settings - Fork 75
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.
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
base: master
Are you sure you want to change the base?
Changes from all commits
c4da5fc
00e55cc
aab240c
a135598
9f4ecd3
0ea1044
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| --- | ||
| '@compiled/babel-plugin': patch | ||
| --- | ||
|
|
||
| Resolve module imports relative to the transformed file when resolving bindings. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,117 @@ | ||
| /** | ||
| * Base resolver, used by tool-specific resolvers. | ||
| * This is used to make sure that packages resolve using the same algorithm as our webpack config | ||
| * (checking for "browser", "main", etc) meaning that we dont need the old root index.js hack anymore | ||
| */ | ||
| const fs = require('fs'); | ||
| const path = require('path'); | ||
|
|
||
| const enhancedResolve = require('enhanced-resolve'); | ||
| const resolveFrom = require('resolve-from'); | ||
|
|
||
| const BASE_DIR = path.resolve(__dirname, '..', '..', '..'); | ||
|
|
||
| const requireCache = new Map(); | ||
|
|
||
| const cached = (key, fn) => { | ||
| if (requireCache.has(key)) { | ||
| return requireCache.get(key); | ||
| } | ||
| const result = fn(); | ||
| requireCache.set(key, result); | ||
| return result; | ||
| }; | ||
|
|
||
| // This is the resolver used by webpack, which we configure similarly | ||
| // to AK website (see ./website/webpack.config.js - "resolve" field) | ||
| const wpResolver = enhancedResolve.ResolverFactory.createResolver({ | ||
| fileSystem: new enhancedResolve.CachedInputFileSystem(fs, 4000), | ||
| useSyncFileSystemCalls: true, | ||
| mainFields: ['browser', 'main'], | ||
| extensions: ['.js', '.ts', '.tsx', '.json', '.d.ts'], | ||
| conditionNames: ['import', 'require', 'types', 'default'], | ||
| }); | ||
|
|
||
| const nodeResolver = enhancedResolve.ResolverFactory.createResolver({ | ||
| fileSystem: new enhancedResolve.CachedInputFileSystem(fs, 4000), | ||
| useSyncFileSystemCalls: true, | ||
| mainFields: ['main'], | ||
| extensions: ['.js', '.ts', '.tsx', '.json', '.d.ts', '.node'], | ||
| conditionNames: ['node', 'import', 'require', 'types', 'default'], | ||
| }); | ||
|
|
||
| const exceptionList = Object.freeze([ | ||
| /@atlassian\/parcel/, | ||
| /@parcel\//, | ||
| 'parcel', | ||
| 'self-published', | ||
| ]); | ||
|
|
||
| const resolveNode = (modulePath, opts) => { | ||
| for (const exception of exceptionList) { | ||
| if ( | ||
| (typeof exception === 'string' && exception === modulePath) || | ||
| (exception instanceof RegExp && exception.test(modulePath)) | ||
| ) { | ||
| return nodeResolver.resolveSync({}, opts.basedir, modulePath); | ||
| } | ||
| } | ||
| return undefined; | ||
| }; | ||
|
|
||
| const resolveModule = (basePath, module) => | ||
| cached(`m:${basePath}/${module}`, () => { | ||
| try { | ||
| return wpResolver.resolveSync({}, basePath, module); | ||
| } catch { | ||
| return undefined; | ||
| } | ||
| }); | ||
|
|
||
| const followSymLink = (file) => | ||
| // Dereference symlinks to ensure we don't create a separate | ||
| // module instance depending on how it was referenced. | ||
| // @link https://github.com/facebook/jest/pull/4761 | ||
| fs.realpathSync(file); | ||
|
|
||
| const resolveRelative = (modulePath /*: string */, opts /*: Object */) => { | ||
| // If resolving relative paths, make sure we use resolveFrom and not resolve | ||
| if (modulePath.startsWith('.') || modulePath.startsWith(path.sep)) { | ||
| return cached(`r:${opts.basedir}/${modulePath}`, () => { | ||
| try { | ||
| // resolveFrom could not "see" .ts/.tsx files | ||
| // the only registered extensions are `.js`, `.json` and `.node` | ||
| return resolveFrom(opts.basedir, modulePath); | ||
| } catch { | ||
| return undefined; | ||
| } | ||
| }); | ||
| } | ||
| return undefined; | ||
| }; | ||
|
|
||
| const resolveNamedEntry = (modulePath /*: string */) => { | ||
| return resolveModule(BASE_DIR, modulePath); | ||
| }; | ||
|
|
||
| const resolveAbsolute = (modulePath /*: string */, opts /*: Object */) => | ||
| resolveModule(opts.basedir, modulePath); | ||
|
|
||
| /** | ||
| * @typedef {Object} Opts - resolver options | ||
| * @property {string} basedir - The base directory of the file importing modulePath | ||
| */ | ||
| /** | ||
| * Base resolver | ||
| * | ||
| * @param {string} modulePath - The module path to be resolved | ||
| * @param {Opts} opts - Resolver options | ||
| */ | ||
| module.exports = function resolver(modulePath /*: string */, opts /*: Object */) { | ||
| return ( | ||
| resolveRelative(modulePath, opts) || | ||
| resolveNamedEntry(modulePath, opts) || | ||
| resolveNode(modulePath, opts) || | ||
| followSymLink(resolveAbsolute(modulePath, opts)) | ||
| ); | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| /** | ||
| * Default resolver used by @compiled/babel-plugin when a custom resolver option | ||
| * is not provided. | ||
| */ | ||
| const path = require('path'); | ||
|
|
||
| const baseResolver = require('./base-resolver'); | ||
|
|
||
| module.exports = { | ||
| resolveSync: (context, request) => { | ||
| return baseResolver(request, { | ||
| basedir: path.dirname(context), | ||
| }); | ||
| }, | ||
| }; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| !node_modules/ | ||
| !node_modules/@compiled-private/ | ||
| !node_modules/@compiled-private/source-relative-responsive/ | ||
| !node_modules/@compiled-private/source-relative-responsive/index.js |
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,5 @@ | ||
| import { join } from 'path'; | ||
|
|
||
| import { transform } from '../../test-utils'; | ||
|
|
||
| describe('xcss prop transformation', () => { | ||
|
|
@@ -81,6 +83,61 @@ describe('xcss prop transformation', () => { | |
| `); | ||
| }); | ||
|
|
||
| it('should resolve primitive xcss imports from package exports relative to the transformed file', () => { | ||
| // Use a fixture package instead of the real Atlaskit package so this test | ||
| // only verifies source-file-relative package resolution, not dependency setup. | ||
| const result = transform( | ||
| ` | ||
| import { Box as AkBox, xcss as akXcss } from '@atlaskit/primitives'; | ||
| import { media as rootMedia } from '@compiled-private/source-relative-responsive'; | ||
| import { media as subpathMedia } from '@compiled-private/source-relative-responsive/media'; | ||
|
|
||
| export const CheckboxList = ({ testId = 'checkbox-list' }) => ( | ||
| <AkBox | ||
| xcss={akXcss({ | ||
| display: 'grid', | ||
| gridTemplateColumns: '1fr 1fr', | ||
| flexWrap: 'wrap', | ||
| flexDirection: 'column', | ||
| [rootMedia.above.sm]: { | ||
| flexWrap: 'nowrap', | ||
| }, | ||
| [subpathMedia.above.sm]: { | ||
| display: 'flex', | ||
| }, | ||
| })} | ||
| testId={testId} | ||
| /> | ||
| ); | ||
| `, | ||
| { filename: join(__dirname, '__fixtures__', 'checkbox-list.jsx') } | ||
| ); | ||
|
|
||
| expect(result).toMatchInlineSnapshot(` | ||
| "import { Box as AkBox, xcss as akXcss } from "@atlaskit/primitives"; | ||
| import { media as rootMedia } from "@compiled-private/source-relative-responsive"; | ||
| import { media as subpathMedia } from "@compiled-private/source-relative-responsive/media"; | ||
| export const CheckboxList = ({ testId = "checkbox-list" }) => ( | ||
| <AkBox | ||
| xcss={akXcss({ | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am a bit confused. After the transformation, should this snapshot look like ?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. other existing test cases are the same, I think it's because the test util it used which made it run in runtime mode. |
||
| display: "grid", | ||
| gridTemplateColumns: "1fr 1fr", | ||
| flexWrap: "wrap", | ||
| flexDirection: "column", | ||
| [rootMedia.above.sm]: { | ||
| flexWrap: "nowrap", | ||
| }, | ||
| [subpathMedia.above.sm]: { | ||
| display: "flex", | ||
| }, | ||
| })} | ||
| testId={testId} | ||
| /> | ||
| ); | ||
| " | ||
| `); | ||
| }); | ||
|
|
||
| it('should allow ternaries', () => { | ||
| const result = transform(` | ||
| import { cssMap } from '@compiled/react'; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this issue specific to compiled plugin? Or does it need to be ported over atlaspack SWC plugins?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
asked AI saying not needed :)
