Skip to content

Commit

Permalink
docs/clean: formally deprecate rollupCommonJSResolveHack (ezolenko#367
Browse files Browse the repository at this point in the history
)

* docs/clean: formally deprecate `rollupCommonJSResolveHack`

- this has had no effect since 6fb0e75, released in 0.30.0
  - that changed the code to always return OS native paths via the NodeJS Path API
    - so setting `rollupCommonJSResolveHack` would make no difference either `true` or `false`
      - effectively, it's as if it's always `true` now

- formally state now that this is deprecated in the docs
  - as well as when that occurred and what it means

- also add a warning in `options` similar to the existing one for `objectHashIgnoreUnknownHack`

- remove the `resolve` dependency as well
  - it turns out something in the devDeps still uses it, so it didn't get fully removed in the `package-lock.json`
  - `resolve` was never needed anyway as we could've used NodeJS's native `path.resolve` or `require.resolve` instead
    - `resolve` was created for `browserify` after all, where one can't use NodeJS APIs
      - but we run on NodeJS and can and already do use NodeJS APIs, including both `path.resolve` _and_ `require.resolve`
  - I actually started this commit just to remove the dep, then realized the entire code path is obsolete

* fix lint error -- resolved can now be const
  • Loading branch information
agilgur5 committed Jun 24, 2022
1 parent b0e3922 commit 74f6761
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 16 deletions.
6 changes: 1 addition & 5 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -156,11 +156,7 @@ See [#108](https://github.com/ezolenko/rollup-plugin-typescript2/issues/108)

* `rollupCommonJSResolveHack`: false

On windows typescript resolver favors POSIX path, while commonjs plugin (and maybe others?) uses native path as module id. This can result in `namedExports` being ignored if rollup happened to use typescript's resolution. Set to true to pass resolved module path through `resolve()` to match up with `rollup-plugin-commonjs`.

`rollup-plugin-commonjs` fixed this in `10.1.0`, so projects using this option who update to new version will be broken again.

This also works around the similar bug affecting code splitting (see [rollup/rollup#3094](https://github.com/rollup/rollup/issues/3094)).
_Deprecated_. OS native paths are now _always_ used since [`0.30.0`](https://github.com/ezolenko/rollup-plugin-typescript2/releases/0.30.0) (see [#251](https://github.com/ezolenko/rollup-plugin-typescript2/pull/251)), so this no longer has any effect -- as if it is always `true`.

* `objectHashIgnoreUnknownHack`: false

Expand Down
15 changes: 12 additions & 3 deletions package-lock.json

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

1 change: 0 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
"@rollup/pluginutils": "^4.1.2",
"find-cache-dir": "^3.3.2",
"fs-extra": "^10.0.0",
"resolve": "^1.20.0",
"tslib": "^2.4.0"
},
"peerDependencies": {
Expand Down
13 changes: 6 additions & 7 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
import { relative, dirname, normalize as pathNormalize, resolve as pathResolve } from "path";
import { relative, dirname, normalize as pathNormalize, resolve } from "path";
import * as tsTypes from "typescript";
import { PluginImpl, PluginContext, InputOptions, OutputOptions, TransformResult, SourceMap, Plugin } from "rollup";
import { normalizePath as normalize } from "@rollup/pluginutils";
import * as _ from "lodash";
import { blue, red, yellow, green } from "colors/safe";
import * as resolve from "resolve";
import findCacheDir from "find-cache-dir";

import { RollupContext } from "./rollupcontext";
Expand Down Expand Up @@ -115,6 +114,9 @@ const typescript: PluginImpl<RPT2Options> = (options) =>
if (pluginOptions.objectHashIgnoreUnknownHack)
context.warn(() => `${yellow("You are using 'objectHashIgnoreUnknownHack' option")}. If you enabled it because of async functions, try disabling it now.`);

if (pluginOptions.rollupCommonJSResolveHack)
context.warn(() => `${yellow("You are using 'rollupCommonJSResolveHack' option")}. This is no longer needed, try disabling it now.`);

if (watchMode)
context.info(`running in watch mode`);
}
Expand Down Expand Up @@ -155,7 +157,7 @@ const typescript: PluginImpl<RPT2Options> = (options) =>

// TODO: use module resolution cache
const result = tsModule.nodeModuleNameResolver(importee, importer, parsedConfig.options, tsModule.sys);
let resolved = result.resolvedModule?.resolvedFileName;
const resolved = result.resolvedModule?.resolvedFileName;

if (!resolved)
return;
Expand All @@ -166,9 +168,6 @@ const typescript: PluginImpl<RPT2Options> = (options) =>
if (resolved.endsWith(".d.ts"))
return;

if (pluginOptions.rollupCommonJSResolveHack)
resolved = resolve.sync(resolved);

context.debug(() => `${blue("resolving")} '${importee}' imported by '${importer}'`);
context.debug(() => ` to '${resolved}'`);

Expand Down Expand Up @@ -332,7 +331,7 @@ const typescript: PluginImpl<RPT2Options> = (options) =>
// invert back to absolute, then make relative to declarationDir
parsedText.sources = parsedText.sources.map(source =>
{
const absolutePath = pathResolve(cachePlaceholder, source);
const absolutePath = resolve(cachePlaceholder, source);
return normalize(relative(declarationDir, absolutePath));
});
entryText = JSON.stringify(parsedText);
Expand Down

0 comments on commit 74f6761

Please sign in to comment.