Skip to content

Commit

Permalink
feat(react): support allowJs customization in the rollup executor (#1…
Browse files Browse the repository at this point in the history 
…6789)
  • Loading branch information
@xiongemi
xiongemi authored May 5, 2023
1 parent 8d35eda commit 575c6a1
Show file tree
Hide file tree
Showing 6 changed files with 15 additions and 44 deletions.
10 changes: 5 additions & 5 deletions docs/generated/packages/rollup/executors/rollup.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,11 @@
"x-completion-glob": "tsconfig.*.json",
"x-priority": "important"
},
"allowJs": {
"type": "boolean",
"description": "Allow JavaScript files to be compiled.",
"default": false
},
"format": {
"type": "array",
"description": "List of module formats to output. Defaults to matching format from tsconfig (e.g. CJS for CommonJS, and ESM otherwise).",
Expand Down Expand Up @@ -154,11 +159,6 @@
"type": "boolean",
"description": "Generate package.json with 'exports' field. This field defines entry points in the package and is used by Node and the TypeScript compiler.",
"default": false
},
"skipTypeField": {

This comment has been minimized.

Sign in to view

Copy link
@ianldgs

ianldgs Jun 15, 2023

Contributor

@xiongemi why was this option removed? :/
can't import lodash from a library built by this executor anymore.

Module not found: Error: Can't resolve 'lodash/defaults' in 'path/to/node_modules/package'
Did you mean 'defaults.js'?
BREAKING CHANGE: The request 'lodash/defaults' failed to resolve only because it was resolved as fully specified
(probably because the origin is strict EcmaScript Module, e. g. a module with javascript mimetype, a '*.mjs' file, or a '*.js' file where the package.json contains '"type": "module"').
The extension in the request is mandatory for it to be fully specified.
Add the extension to the request.

This comment has been minimized.

Sign in to view

Copy link
@xiongemi

xiongemi Jun 15, 2023

Author Collaborator

could you use https://nx.dev/packages/rollup/executors/rollup#format to set format to CJS?

This comment has been minimized.

Sign in to view

Copy link
@ianldgs

ianldgs Jun 16, 2023 •
edited
Loading

Contributor

Yes, could, but would rather not.
In the end this made us realise that the lib didn't really need lodash, so just got rid of it.
But would be nice to have some sort of communication next time, like adding this to the list of breaking changes, or first deprecating it. Or maybe having a migration to set it to CJS if skipTypeField was found.
Thanks for the reply!
"type": "boolean",
"description": "Prevents 'type' field from being added to compiled package.json file. Only use this if you are having an issue with this field.",
"default": false
}
},
"required": ["tsConfig", "main", "outputPath"],
Expand Down
26 changes: 0 additions & 26 deletions packages/rollup/src/executors/rollup/lib/update-package-json.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -265,30 +265,4 @@ describe('updatePackageJson', () => {
spy.mockRestore();
});
});

describe('skipTypeField', () => {
it('should not add "type" field in package.json', () => {
const spy = jest.spyOn(utils, 'writeJsonFile');

updatePackageJson(
{
...commonOptions,
format: ['esm'],
skipTypeField: true,
},
sharedContext,
{ type: 'app', name: 'test', data: {} as any },
[],
{} as unknown as PackageJson
);

expect(utils.writeJsonFile).toHaveBeenCalledWith(expect.anything(), {
main: './index.js',
module: './index.js',
types: './index.d.ts',
});

spy.mockRestore();
});
});
});
4 changes: 1 addition & 3 deletions packages/rollup/src/executors/rollup/lib/update-package-json.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,7 @@ export function updatePackageJson(
exports['.']['require'] = './index.cjs';
}

if (!options.skipTypeField) {
packageJson.type = options.format.includes('esm') ? 'module' : 'commonjs';
}
packageJson.type = options.format.includes('esm') ? 'module' : 'commonjs';

// Support for older TS versions < 4.5
packageJson.types = types;
Expand Down
6 changes: 3 additions & 3 deletions packages/rollup/src/executors/rollup/rollup.impl.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -313,13 +313,13 @@ export function createRollupOptions(

function createTsCompilerOptions(
config: ts.ParsedCommandLine,
dependencies,
options
dependencies: DependentBuildableProjectNode[],
options: NormalizedRollupExecutorOptions
) {
const compilerOptionPaths = computeCompilerOptionsPaths(config, dependencies);
const compilerOptions = {
rootDir: options.projectRoot,
allowJs: false,
allowJs: options.allowJs,
declaration: true,
paths: compilerOptionPaths,
};
Expand Down
3 changes: 1 addition & 2 deletions packages/rollup/src/executors/rollup/schema.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ export interface Globals {
export interface RollupExecutorOptions {
outputPath: string;
tsConfig: string;
allowJs?: boolean;
project: string;
main: string;
outputFileName?: string;
Expand All @@ -29,7 +30,5 @@ export interface RollupExecutorOptions {
format?: string[];
compiler?: 'babel' | 'tsc' | 'swc';
javascriptEnabled?: boolean;
// TODO(jack): remove this for Nx 16
skipTypeField?: boolean;
generateExportsField?: boolean;
}
10 changes: 5 additions & 5 deletions packages/rollup/src/executors/rollup/schema.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@
"x-completion-glob": "tsconfig.*.json",
"x-priority": "important"
},
"allowJs": {
"type": "boolean",
"description": "Allow JavaScript files to be compiled.",
"default": false
},
"format": {
"type": "array",
"description": "List of module formats to output. Defaults to matching format from tsconfig (e.g. CJS for CommonJS, and ESM otherwise).",
Expand Down Expand Up @@ -141,11 +146,6 @@
"type": "boolean",
"description": "Generate package.json with 'exports' field. This field defines entry points in the package and is used by Node and the TypeScript compiler.",
"default": false
},
"skipTypeField": {
"type": "boolean",
"description": "Prevents 'type' field from being added to compiled package.json file. Only use this if you are having an issue with this field.",
"default": false
}
},
"required": ["tsConfig", "main", "outputPath"],
Expand Down

1 comment on commit 575c6a1

@vercel
Copy link

@vercel vercel bot commented on 575c6a1 May 5, 2023

Choose a reason for hiding this comment

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

Successfully deployed to the following URLs:

nx-dev – ./

nx-dev-nrwl.vercel.app
nx.dev
nx-five.vercel.app
nx-dev-git-master-nrwl.vercel.app

Please sign in to comment.