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

Improve import resolution logic #519

Merged
merged 1 commit into from
Jun 19, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
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
13 changes: 0 additions & 13 deletions src/commands/build-util.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import type CSSModuleLoader from 'css-modules-loader-core';
import type {EventEmitter} from 'events';
import execa from 'execa';
import {statSync} from 'fs';
import npmRunPath from 'npm-run-path';
import path from 'path';
import {
BuildScript,
SnowpackConfig,
Expand All @@ -16,17 +14,6 @@ export function checkIsPreact(filePath: string, contents: string) {
return filePath.endsWith('.jsx') && IS_PREACT.test(contents);
}

export function isDirectoryImport(fileLoc: string, spec: string): boolean {
const importedFileOnDisk = path.resolve(path.dirname(fileLoc), spec);
try {
const stat = statSync(importedFileOnDisk);
return stat.isDirectory();
} catch (err) {
// file doesn't exist, that's fine
}
return false;
}

export function wrapImportMeta({
code,
hmr,
Expand Down
7 changes: 5 additions & 2 deletions src/commands/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -291,8 +291,11 @@ export async function command(commandOptions: CommandOptions) {
// We treat ".proxy.js" files special: we need to make sure that they exist on disk
// in the final build, so we mark them to be written to disk at the next step.
if (resolvedImportUrl.endsWith('.proxy.js')) {
const resolvedUrl = path.resolve(path.dirname(outPath), spec);
allProxiedFiles.add(resolvedUrl);
allProxiedFiles.add(
resolvedImportUrl.startsWith('/')
Copy link
Owner Author

Choose a reason for hiding this comment

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

bug fix: we weren't resolving /_dist_/ URLs properly, since they get handled as absolute paths by path.resolve

Copy link
Collaborator

Choose a reason for hiding this comment

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

👏

? path.resolve(cwd, spec)
: path.resolve(path.dirname(outPath), spec),
);
}
return resolvedImportUrl;
}
Expand Down
54 changes: 37 additions & 17 deletions src/commands/import-resolver.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import {Stats, statSync} from 'fs';
import path from 'path';
import {SnowpackConfig} from '../config';
import {findMatchingMountScript} from '../util';
import {isDirectoryImport} from './build-util';
import srcFileExtensionMapping from './src-file-extension-mapping';

const cwd = process.cwd();
Copy link
Owner Author

Choose a reason for hiding this comment

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

how do you feel about adding cwd to the config object? We do this in a few different places around the codebase, but it would be good to start moving towards a configurable cwd.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think that’s a great idea.

const URL_HAS_PROTOCOL_REGEX = /^\w:\/\./;

interface ImportResolverOptions {
Expand All @@ -14,6 +14,36 @@ interface ImportResolverOptions {
config: SnowpackConfig;
}

/** Perform a file disk lookup for the requested import specifier. */
export function getImportStats(dirLoc: string, spec: string): Stats | false {
const importedFileOnDisk = path.resolve(dirLoc, spec);
try {
return statSync(importedFileOnDisk);
} catch (err) {
// file doesn't exist, that's fine
}
return false;
}

/** Resolve an import based on the state of the file/folder found on disk. */
function resolveSourceSpecifier(spec: string, stats: Stats | false, isBundled: boolean) {
if (stats && stats.isDirectory()) {
spec = spec + '/index.js';
} else if (!stats && !spec.endsWith('.js')) {
spec = spec + '.js';
}
const ext = path.extname(spec).substr(1);
const extToReplace = srcFileExtensionMapping[ext];
if (extToReplace) {
spec = spec.replace(new RegExp(`${ext}$`), extToReplace);
}
if (!isBundled && (extToReplace || ext) !== 'js') {
spec = spec + '.proxy.js';
}

return spec;
}

/**
* Create a import resolver function, which converts any import relative to the given file at "fileLoc"
* to a proper URL. Returns false if no matching import was found, which usually indicates a package
Expand All @@ -36,24 +66,14 @@ export function createImportResolver({
let mountScript = findMatchingMountScript(config.scripts, spec);
if (mountScript) {
let {fromDisk, toUrl} = mountScript.args;
const importStats = getImportStats(cwd, spec);
spec = resolveSourceSpecifier(spec, importStats, isBundled);
spec = spec.replace(fromDisk, toUrl);
return spec;
}
if (spec.startsWith('/') || spec.startsWith('./') || spec.startsWith('../')) {
const ext = path.extname(spec).substr(1);
if (!ext) {
if (isDirectoryImport(fileLoc, spec)) {
return spec + '/index.js';
} else {
return spec + '.js';
}
}
const extToReplace = srcFileExtensionMapping[ext];
if (extToReplace) {
spec = spec.replace(new RegExp(`${ext}$`), extToReplace);
}
if (!isBundled && (extToReplace || ext) !== 'js') {
spec = spec + '.proxy.js';
}
const importStats = getImportStats(path.dirname(fileLoc), spec);
spec = resolveSourceSpecifier(spec, importStats, isBundled);
return spec;
}
if (dependencyImportMap.imports[spec]) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default {"MODE":"production","NODE_ENV":"production"};
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default "Button";
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
body { color: red }
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@

const code = "body { color: red }";

const styleEl = document.createElement("style");
const codeEl = document.createTextNode(code);
styleEl.type = 'text/css';

styleEl.appendChild(codeEl);
document.head.appendChild(styleEl);
16 changes: 16 additions & 0 deletions test/build/resolve-imports/expected-build/_dist_/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import {flatten} from "/web_modules/array-flatten.js";
console.log(flatten);
import sort2 from "./sort.js";
import sort_ from "/_dist_/sort.js";
import sort__ from "/_dist_/sort.js";
console.log(sort2, sort_, sort__);
import components2 from "./components/index.js";
import components_ from "./components/index.js";
import components__ from "./components/index.js";
import components___ from "/_dist_/components/index.js";
import components____ from "/_dist_/components/index.js";
import components_____ from "/_dist_/components/index.js";
console.log(components2, components_, components__, components___, components____, components_____);
import styles from "./components/style.css.proxy.js";
import styles_ from "/_dist_/components/style.css.proxy.js";
console.log(styles, styles_);
1 change: 1 addition & 0 deletions test/build/resolve-imports/expected-build/_dist_/sort.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default (arr) => arr.sort();
20 changes: 20 additions & 0 deletions test/build/resolve-imports/expected-build/index.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width, initial-scale=1" />
<meta name="description" content="Web site created using create-snowpack-app" />
<title>Snowpack App</title>
</head>
<body>
<script type="module" src="/_dist_/index.js"></script>
<!--
This HTML file is a template.
If you open it directly in the browser, you will see an empty page.
You can add webfonts, meta tags, or analytics to this file.
The build step will place the bundled scripts into the <body> tag.
To begin the development, run `npm start` or `yarn start`.
To create a production bundle, use `npm run build` or `yarn build`.
-->
</body>
</html>
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/**
* Flatten an array indefinitely.
*/
function flatten(array) {
var result = [];
$flatten(array, result);
return result;
}
/**
* Internal flatten function recursively passes `result`.
*/
function $flatten(array, result) {
for (var i = 0; i < array.length; i++) {
var value = array[i];
if (Array.isArray(value)) {
$flatten(value, result);
}
else {
result.push(value);
}
}
}

export { flatten };
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"imports": {
"array-flatten": "./array-flatten.js"
}
}
21 changes: 21 additions & 0 deletions test/build/resolve-imports/node_modules/array-flatten/LICENSE

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

43 changes: 43 additions & 0 deletions test/build/resolve-imports/node_modules/array-flatten/README.md

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.

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.

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.

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.

Loading