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

Add --package_json_entry_names command line flag #2600

Closed
wants to merge 1 commit into from
Closed

Add --package_json_entry_names command line flag #2600

wants to merge 1 commit into from

Conversation

anmonteiro
Copy link
Contributor

This PR builds on #2598 and adds a command line flag that allows specifying the ordered entries which RewriteJsonToModule should look for when computing the main entry in package.json files.

refs #2433

@dimvar
Copy link
Contributor

dimvar commented Aug 4, 2017

FYI, I won't have time to look at this today.

@anmonteiro
Copy link
Contributor Author

Thanks for setting expectations. No rush.

@dimvar
Copy link
Contributor

dimvar commented Aug 4, 2017

Also, you may be amused to know that your previous PR crashed the tool we use to bring PRs internally, because of the unicode character ó in your name :) So I had to edit the script to add the name without the accent.

Consider using the name without the accent in your git settings to avoid this in the future. (Up to you, we can work around this, just thought I'd let you know.)

@anmonteiro
Copy link
Contributor Author

that's just too funny 😃

I can do that in my git settings, I should probably amend this commit then.

@anmonteiro
Copy link
Contributor Author

Just amended this commit to not include the ó character in the author line.

@alexeagle
Copy link
Contributor

+1 we need this for Angular as well, @jasonaden and I built locally and tested today.

Copy link
Collaborator

@ChadKillingsworth ChadKillingsworth left a comment

Choose a reason for hiding this comment

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

I'd like to land #2604 first. Can we change this to default to the list browser, module, main? That's what webpack uses.

@anmonteiro
Copy link
Contributor Author

@ChadKillingsworth fixed and squashed commits. Any other notes?

@Option(name = "--package_json_entry_names",
usage = "Ordered list of entries to look for in package.json files when processing "
+ "modules with the NODE module resolution strategy (i.e. esnext:main,browser,main). "
+ "Defaults to a list with a single entry, \"main\"."
Copy link
Collaborator

Choose a reason for hiding this comment

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

Need to update this comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nice catch. fixed.

* Needed by {@link RewriteJsonToModule}, but defined here because RewriteJsonToModule is not
* part of the core build.
*/
public static final String PACKAGE_JSON_MAIN = "main";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is not actually used anywhere else, since we generalized package.json fields.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Then I'd just remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's removed. My comment is to justify the deletion.

@blickly blickly closed this in a84c1f1 Aug 8, 2017
jelbourn added a commit to jelbourn/components that referenced this pull request Sep 12, 2017
Before this change, all of `@angular/material` was bundled into a
single, flat JS file.

After this change, each top-level component (i.e., each directory under
`src/lib`) will be have its own JS file (and d.ts and metadata.json).

The benefit of this change is that current versions of webpack will be
able to do a much better job of only including the code an app is using.

It also allows (but does not require) people to explicitly import from specific components,
e.g.
```
import {MdButtonModule} from '@angular/material/button';
``

For development _within Angular Material itself_ we have to use these
deeper imports because the root `@angular/material` doesn't exist until
we build the relase package.

Additionally, this PR adds a `tsconfig.json` for each dir under `src/`
with the correct path mappings for that directory, so that IDEs will
correctly generate imports.

Closure Compiler CI check is also disabled by this PR; it cannot work
with the new package structure until a release with
google/closure-compiler#2600 is made available.
jelbourn added a commit to jelbourn/components that referenced this pull request Sep 12, 2017
Before this change, all of `@angular/material` was bundled into a
single, flat JS file.

After this change, each top-level component (i.e., each directory under
`src/lib`) will be have its own JS file (and d.ts and metadata.json).

The benefit of this change is that current versions of webpack will be
able to do a much better job of only including the code an app is using.

It also allows (but does not require) people to explicitly import from specific components,
e.g.
```
import {MdButtonModule} from '@angular/material/button';
``

For development _within Angular Material itself_ we have to use these
deeper imports because the root `@angular/material` doesn't exist until
we build the relase package.

Additionally, this PR adds a `tsconfig.json` for each dir under `src/`
with the correct path mappings for that directory, so that IDEs will
correctly generate imports.

Closure Compiler CI check is also disabled by this PR; it cannot work
with the new package structure until a release with
google/closure-compiler#2600 is made available.
jelbourn added a commit to jelbourn/components that referenced this pull request Sep 12, 2017
Before this change, all of `@angular/material` was bundled into a
single, flat JS file.

After this change, each top-level component (i.e., each directory under
`src/lib`) will be have its own JS file (and d.ts and metadata.json).

The benefit of this change is that current versions of webpack will be
able to do a much better job of only including the code an app is using.

It also allows (but does not require) people to explicitly import from specific components,
e.g.
```
import {MdButtonModule} from '@angular/material/button';
``

For development _within Angular Material itself_ we have to use these
deeper imports because the root `@angular/material` doesn't exist until
we build the relase package.

Additionally, this PR adds a `tsconfig.json` for each dir under `src/`
with the correct path mappings for that directory, so that IDEs will
correctly generate imports.

Closure Compiler CI check is also disabled by this PR; it cannot work
with the new package structure until a release with
google/closure-compiler#2600 is made available.
jelbourn added a commit to jelbourn/components that referenced this pull request Sep 12, 2017
Before this change, all of `@angular/material` was bundled into a
single, flat JS file.

After this change, each top-level component (i.e., each directory under
`src/lib`) will be have its own JS file (and d.ts and metadata.json).

The benefit of this change is that current versions of webpack will be
able to do a much better job of only including the code an app is using.

It also allows (but does not require) people to explicitly import from specific components,
e.g.
```
import {MdButtonModule} from '@angular/material/button';
``

For development _within Angular Material itself_ we have to use these
deeper imports because the root `@angular/material` doesn't exist until
we build the relase package.

Additionally, this PR adds a `tsconfig.json` for each dir under `src/`
with the correct path mappings for that directory, so that IDEs will
correctly generate imports.

Closure Compiler CI check is also disabled by this PR; it cannot work
with the new package structure until a release with
google/closure-compiler#2600 is made available.
jelbourn added a commit to jelbourn/components that referenced this pull request Sep 13, 2017
Before this change, all of `@angular/material` was bundled into a
single, flat JS file.

After this change, each top-level component (i.e., each directory under
`src/lib`) will be have its own JS file (and d.ts and metadata.json).

The benefit of this change is that current versions of webpack will be
able to do a much better job of only including the code an app is using.

It also allows (but does not require) people to explicitly import from specific components,
e.g.
```
import {MdButtonModule} from '@angular/material/button';
``

For development _within Angular Material itself_ we have to use these
deeper imports because the root `@angular/material` doesn't exist until
we build the relase package.

Additionally, this PR adds a `tsconfig.json` for each dir under `src/`
with the correct path mappings for that directory, so that IDEs will
correctly generate imports.

Closure Compiler CI check is also disabled by this PR; it cannot work
with the new package structure until a release with
google/closure-compiler#2600 is made available.
jelbourn added a commit to jelbourn/components that referenced this pull request Sep 13, 2017
Before this change, all of `@angular/material` was bundled into a
single, flat JS file.

After this change, each top-level component (i.e., each directory under
`src/lib`) will be have its own JS file (and d.ts and metadata.json).

The benefit of this change is that current versions of webpack will be
able to do a much better job of only including the code an app is using.

It also allows (but does not require) people to explicitly import from specific components,
e.g.
```
import {MdButtonModule} from '@angular/material/button';
``

For development _within Angular Material itself_ we have to use these
deeper imports because the root `@angular/material` doesn't exist until
we build the relase package.

Additionally, this PR adds a `tsconfig.json` for each dir under `src/`
with the correct path mappings for that directory, so that IDEs will
correctly generate imports.

Closure Compiler CI check is also disabled by this PR; it cannot work
with the new package structure until a release with
google/closure-compiler#2600 is made available.
mmalerba pushed a commit to angular/components that referenced this pull request Sep 14, 2017
Before this change, all of `@angular/material` was bundled into a
single, flat JS file.

After this change, each top-level component (i.e., each directory under
`src/lib`) will be have its own JS file (and d.ts and metadata.json).

The benefit of this change is that current versions of webpack will be
able to do a much better job of only including the code an app is using.

It also allows (but does not require) people to explicitly import from specific components,
e.g.
```
import {MdButtonModule} from '@angular/material/button';
``

For development _within Angular Material itself_ we have to use these
deeper imports because the root `@angular/material` doesn't exist until
we build the relase package.

Additionally, this PR adds a `tsconfig.json` for each dir under `src/`
with the correct path mappings for that directory, so that IDEs will
correctly generate imports.

Closure Compiler CI check is also disabled by this PR; it cannot work
with the new package structure until a release with
google/closure-compiler#2600 is made available.
josephperrott pushed a commit to josephperrott/components that referenced this pull request Sep 15, 2017
Before this change, all of `@angular/material` was bundled into a
single, flat JS file.

After this change, each top-level component (i.e., each directory under
`src/lib`) will be have its own JS file (and d.ts and metadata.json).

The benefit of this change is that current versions of webpack will be
able to do a much better job of only including the code an app is using.

It also allows (but does not require) people to explicitly import from specific components,
e.g.
```
import {MdButtonModule} from '@angular/material/button';
``

For development _within Angular Material itself_ we have to use these
deeper imports because the root `@angular/material` doesn't exist until
we build the relase package.

Additionally, this PR adds a `tsconfig.json` for each dir under `src/`
with the correct path mappings for that directory, so that IDEs will
correctly generate imports.

Closure Compiler CI check is also disabled by this PR; it cannot work
with the new package structure until a release with
google/closure-compiler#2600 is made available.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants